-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Film 100 #109
base: master
Are you sure you want to change the base?
Conversation
src/App.js
Outdated
apiKey={ALGOLIA_SEARCH_KEY} | ||
indexName="roles" | ||
> | ||
<Index indexName="names" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is removing the functionality with the virtual menu to hide in autocomplete non public items. Probably just a funky merge is my guess. But we need to keep the virutal menu and Current Refinements and Configure components.
Yeah, the build failed because it said Virtual Menu, and the two other
algolia imported components weren’t being used. Not sure what happened, I
don’t think I deleted anything manually?
…On Sat, Feb 24, 2018 at 5:20 PM Kyle Potts ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/App.js
<#109 (comment)>
:
> + <div>
+ <Row>
+ <Col xs>
+ <Link to="/"><img src={Logo} className="logo" alt="Film Indy Logo" /></Link>
+ </Col>
+ { location.pathname !== '/' &&
+ <Col xs>
+ <Card className="menuSearchCard">
+ <div style={{ display: 'flex', flexDirection: 'row' }}>
+ <SearchIcon className="searchIcon" />
+ <InstantSearch
+ appId={ALGOLIA_APP_ID}
+ apiKey={ALGOLIA_SEARCH_KEY}
+ indexName="roles"
+ >
+ <Index indexName="names" />
This is removing the functionality with the virtual menu to hide in
autocomplete non public items. Probably just a funky merge is my guess. But
we need to keep the virutal menu and Current Refinements and Configure
components.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#109 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJXELYel-iQSQqLAW3SQw_jjBkt19ZLTks5tYIsXgaJpZM4SSEHs>
.
|
src/App.js
Outdated
<Index indexName="names" /> | ||
<Index indexName="vendors" /> | ||
<Index indexName="locations" /> | ||
<Index indexName="locationTypes" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ejmudrak there is a major difference here. This is where the virtual menu is being used. In this PR, it is being removed. It shouldn't be.
I’ll add it back later.
I set the breakpoint to 400 for mobile, so I’ll just need to change it to
something bigger so that looks better. Adding the grid to everything will
take much more time than what I’ve done, so I can do that next week if we
decide it’s worthwhile
…On Sat, Feb 24, 2018 at 5:26 PM Kyle Potts ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/App.js
<#109 (comment)>
:
> + <Link to="/"><img src={Logo} className="logo" alt="Film Indy Logo" /></Link>
+ </Col>
+ { location.pathname !== '/' &&
+ <Col xs>
+ <Card className="menuSearchCard">
+ <div style={{ display: 'flex', flexDirection: 'row' }}>
+ <SearchIcon className="searchIcon" />
+ <InstantSearch
+ appId={ALGOLIA_APP_ID}
+ apiKey={ALGOLIA_SEARCH_KEY}
+ indexName="roles"
+ >
+ <Index indexName="names" />
+ <Index indexName="vendors" />
+ <Index indexName="locations" />
+ <Index indexName="locationTypes" />
@ejmudrak <https://github.com/ejmudrak> there is a major difference here.
This is where the virtual menu is being used. In this PR, it is being
removed. It shouldn't be.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#109 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJXELbHiIx7GADvlNwGwJ8wg4irFhzgRks5tYIyMgaJpZM4SSEHs>
.
|
Grid shouldn't take that much time its 100% easier then setting manual breakpoint. I'll push something up in a bit to give you an example. |
@ejmudrak I think ~850 would be a better breakpoint size. I took a second look and it seems fine for the profile pages to have not use the responsive grid. I also noticed two other things.
|
If you are interested in what the grid style would look like #112 |
@ejmudrak We can talk about this in the meeting, but I pulled your branch into my branch with cleans up the Navbar and the base search page. So once thats all finished up I can just merge that and we can close this branch. |
Great, I just added Rows to the search page items myself so that they're spaced correctly. I'll go ahead and commit then stop so we don't write overlapping code! |
Uh lets wait till the meeting. I did a lot of changes to the search page and I'll guess they will conflict. |
I hate using !important in css, but to make it responsive, I went for it.