-
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
Refactor Header component to function #1241
Conversation
This pull request is automatically deployed with Now. Latest deployment for this branch: https://venia-git-supernova-979header.magento-research1.now.sh |
1e95ef8
to
afe2761
Compare
get searchIcon() { | ||
return <Icon src={SearchIcon} />; | ||
} | ||
const SearchBar = React.lazy(() => import('src/components/SearchBar')); |
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'm new to React.lazy
. How can I see that this is indeed lazily loading? What affect should it have in the browser? I'm not seeing a new chunk/request being made which is confusing me :D
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.
Looks good! I only had one question/comment but probably not worth holding up the PR.
feat: centralize configuration fixup porting lambda to new config fixup config management from sender, not receiver refactor: Configuration class docs: unified config doc draft
Description
This PR updates the
Header/header.js
component to be a function instead of a class.It was part of the transition to Peregrine / Hooks, but this component is very simple and doesn't need to make use of any hooks at all. As a result, nothing was moved to Peregrine in this PR.
Related Issue
Closes #979 .
Verification Steps
How Have YOU Tested this?
yarn test
Screenshots / Screen Captures (if appropriate):
The Header component:
Proposed Labels for Change Type/Package
Checklist: