-
Notifications
You must be signed in to change notification settings - Fork 13
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
feat(MobileSearch): Updated mobile search to the new algolia view #1797
Conversation
@@ -33,6 +33,11 @@ | |||
<div class="m-menu__language m-menu__section"></div> | |||
</div> | |||
<div class="m-menu__search" data-nav="search"> | |||
<%= render SiteWeb.PageView, "_search_bar.html", conn: @conn, placeholder: "Search for routes, info, and more", id: "header-mobile" %> | |||
<.algolia_autocomplete | |||
id="header-desktop" |
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.
let's maintain the "header-mobile"
id here
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.
Thanks for upload to dev-blue. Has interacting with it there worked ok on your phone?
I tried on my iPhone and it was mostly fine, I noticed a few things:
- This code makes it so that opening the search menu automatically brings your focus to the search input. I'm guessing the selector might need to be updated to maintain that feature here.
- The old version changes the header background color to a darker blue when the search gets opened. That code doesn't seem to work equally well with the new search bar - it reverts back to the light blue when there's a query with too few characters to render the search results.
- After I click a result (or after I press enter) and get taken to the new page, I can't seem to scroll.... Not sure if that's a consistent thing or not though.
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.
- The custom styles used in the header desktop search bar don't automatically get applied here, did you see it look different? Anyway, adding the
#header-mobile
selector here should do it. - After I click a result and get taken to the new page, I am unable to scroll the page. Can you look into this? Might need to use the Simulator if it's not happening on all phones.
|
|
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.
Please QA against prod or dev -- the tablet view needs to maintain disabled scrolling of the page body.
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.
📱 I think it'll work, thanks! Can you update to the latest master / resolve the conflict so the tests can run?
Summary of changes
Replaced the mobile search input with the new algolia search component.
Functionally nothing should have changed from the users point of view
Asana Ticket: AutocompleteJS v1.0 | Mobile header
General checks
New UI, or substantial UI changes
New endpoints, or non-trivial changes to current endpoints