-
Notifications
You must be signed in to change notification settings - Fork 6
moved "Upload Hours" button from avatar menu to ADMIN menu #2246
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
| </ListItem> | ||
| <Collapse in={adminOpen} timeout="auto" unmountOnExit> | ||
| {createListJsx(adminLinks, true)} | ||
| {isAdmin && ( |
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 code was moved here from below.
I had to customize the styling of the button to match the look of the links in the ADMIN menu.
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.
Any reason not to just make it a <ListItem> like the others?
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 didn't realize I could attach an onClick to a ListItem. I pushed that change.
| {isAdmin && ( | ||
| <MenuItem | ||
| onClick={() => { | ||
| closeAvatarMenu(); |
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 didn't need the call to closeAvatarMenu above since the button is no longer in that menu.
vhscom
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.
Looks good to me. I see button prop on ListItem is deprecated.
I removed those deprecated props. |
jackkeller
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.
Approved
No description provided.