-
Notifications
You must be signed in to change notification settings - Fork 687
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Header: Replace Suspense Fallback for SearchBar #1351
Conversation
This pull request is automatically deployed with Now. Latest deployment for this branch: https://venia-git-feature-search-suspense-component.magento-research1.now.sh |
|
QA Pass. @sirugh If new changes are good, please go ahead and merge this one. |
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.
Just one request before we merge.
|
||
.input { | ||
composes: input from '../TextInput/textInput.css'; | ||
max-width: 24rem; |
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 value comes from searchBar.css
as we want the placeholder to look identical to the searchBar
once it loads. I think this should be a constant somewhere shared between both css files so we only have to update in one place in the future.
} | ||
|
||
.loader:before { | ||
color: rgb(var(--venia-grey)); |
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.
Not to nitpick but on my screen this is super light and really difficult to see. If this value is from the mocks, fine, but maybe it needs to be darker.
Description
Fix edge case scenario on slow devices where Suspense temporarily loads a fallback component while the SearchBar component is being loaded.
Related Issue
Closes #1273
Verification Steps
/search.html?query=top
How have YOU tested this?
Ran
yarn test
and manually checked above stepsScreenshots / Screen Captures (if appropriate):
Proposed Labels for Change Type/Package
Checklist: