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

Css improvements 2 #363

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Css improvements 2 #363

wants to merge 4 commits into from

Conversation

cirex-web
Copy link
Contributor

@cirex-web cirex-web commented Dec 14, 2023

  • Fixed manifest.json theme_color (added # sign)
  • Decreased padding on mobile (I added way too much)
  • Made search box wrapping dynamic (not based on breakpoints anymore)

If there was too long of a text before it would do this
Pasted Graphic 11
But now the search bar will wrap to the next line
Pasted Graphic 12

  • Scrolling is actually full-page scroll (rather than just everything-but-bottom-tab scroll) (so mobile browsers can actually detect that the page is taller than screen height - otherwise nav/url bar mide not autohide)
  • Header is now sticky (primarily so the search bar is always visible)
  • Minor changes to grid breakpoints
  • Changed Locations-container padding to margin (but in doing so I had to use -webkit-fill-available to make sure container doesn’t collapse when search results are empty - do let me know if there’s a better way!)
  • Removed some React CSS boilerplate
  • Added back max-width on Locations-container - any reason why this was reverted in this commit?

Open for feedback!

Copy link
Member

@GhostOf0days GhostOf0days left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current feedback: Overall, not a fan of the new list page look. From a UI perspective, I like the original look more, and I'm not sure we should change it. In regards to the search bar, the changes may or may not be good additions. I'm in favor of dynamic wrapping and neutral towards a stickied header, but this decision should be based on user feedback (most UI changes for old features should be on input from design committee or user feedback). We can collect some input at work sessions/meetings early next semester in regards to the search bar changes. Also, nice catch in the manifest file. Lastly, scrolling is a good change, but requires testing across android and iOS (at work sessions/meetings).

@cirex-web
Copy link
Contributor Author

Overall, not a fan of the new list page look. From a UI perspective, I like the original look more, and I'm not sure we should change it.

I don't remember changing that much - what specifically did you have in mind when writing this? I did revert the grid spacing though - it was a bit too much.

In regards to the search bar, the changes may or may not be good additions. I'm in favor of dynamic wrapping and neutral towards a stickied header, but this decision should be based on user feedback (most UI changes for old features should be on input from design committee or user feedback).

Alright - I guess we'll put a hold on this for now.

Lastly, scrolling is a good change, but requires testing across android and iOS (at work sessions/meetings).

Okay - let me know when we can test.

@GhostOf0days
Copy link
Member

As for the new list page look, it felt more squished into the center. I'll explain what I mean after #368 and eatery card skeletons from #329 are merged (in that order). We can test around then.

@GhostOf0days GhostOf0days mentioned this pull request Jan 15, 2024
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