-
Notifications
You must be signed in to change notification settings - Fork 17
refactor: initially render bell icon server side #310
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
Conversation
c674bac to
3b47950
Compare
3b47950 to
289896f
Compare
| <div className="ab-item ab-empty-item" tabIndex={ 0 }> | ||
| <ShortcutProvider | ||
| className={ classNames( [ | ||
| 'notifications', | ||
| isActive ? 'active' : '', | ||
| ] ) } | ||
| > | ||
| <NotificationHubIcon | ||
| toggle={ toggleDrawer } | ||
| isActive={ isActive } | ||
| /> | ||
| <Drawer | ||
| instance={ drawerRef } | ||
| focus={ () => setIsActive( true ) } | ||
| blur={ () => setIsActive( false ) } | ||
| /> | ||
| </ShortcutProvider> | ||
| </div> |
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.
Wrapping this in div.ab-item.ab-empty-item creates the same HTML structure as WordPress creates for the menu item.
|
|
||
| /* the bell icon */ | ||
| .ab-item { | ||
| .hub-icon { |
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.
Changing this name to reflect it's purpose, and the case ab-item has been moved up higher in the DOM structure.
| appearance: none; | ||
| height: 32px; | ||
| display: block; | ||
| padding: 0 10px; |
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 ab-empty-item class provides the padding. The ab-empty-item class is applied to menu items that do not have a link.
0291fab to
5b9f64e
Compare
erikyo
left a comment
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.
LGTM!
What?
Render the bell icon on the server.
Why?
The bell icon would appear after the JavaScript loaded and cause the icon to jump on every page transition.
How?
@erikyo helped me with the code required to render the bell in the admin bar.
TODO