Skip to content
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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Film 100 #109

wants to merge 8 commits into from

Conversation

ejmudrak
Copy link
Contributor

I hate using !important in css, but to make it responsive, I went for it.

@ejmudrak ejmudrak requested a review from kylepotts February 24, 2018 22:05
src/App.js Outdated
apiKey={ALGOLIA_SEARCH_KEY}
indexName="roles"
>
<Index indexName="names" />
Copy link
Contributor

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.

@ejmudrak
Copy link
Contributor Author

ejmudrak commented Feb 24, 2018 via email

@kylepotts
Copy link
Contributor

2018-02-24-172152_778x656_scrot
2018-02-24-172307_686x659_scrot

Things still look a little wonky

src/App.js Outdated
<Index indexName="names" />
<Index indexName="vendors" />
<Index indexName="locations" />
<Index indexName="locationTypes" />
Copy link
Contributor

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.

@ejmudrak
Copy link
Contributor Author

ejmudrak commented Feb 24, 2018 via email

@kylepotts
Copy link
Contributor

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.

@kylepotts
Copy link
Contributor

kylepotts commented Feb 25, 2018

@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.

  1. There appears to be two scroll bars on the home page now (overflow issue? )
  2. The View { Profile, Location, Vendor} cards no long have spacing between them.

@kylepotts
Copy link
Contributor

If you are interested in what the grid style would look like #112

@kylepotts
Copy link
Contributor

@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.

@ejmudrak
Copy link
Contributor Author

ejmudrak commented Mar 4, 2018

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!

@kylepotts
Copy link
Contributor

kylepotts commented Mar 4, 2018

Uh lets wait till the meeting. I did a lot of changes to the search page and I'll guess they will conflict.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants