-
Notifications
You must be signed in to change notification settings - Fork 169
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
Create separate panel for Delegates #9332
Conversation
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.
Technically okay but you might want to implement Kevin's suggested changes before merging
Done |
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.
Tiny suggestion that really doesn't matter in the grand scheme of things. Feel free to implement it, or to ignore it and merge as-is :D
<Header as="h2">Important Links</Header> | ||
<List> | ||
{IMPORTANT_LINKS.map(({ title, link }) => ( | ||
<ListItemLink key={title.replace(/ /g, '')} title={title} link={link} /> |
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.
Why not just take the link
URL itself as a key?
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.
That's cleaner, accepted the suggestion. :-)
Currently, most of the panels are already migrated to panel. But there are two panels which are not yet migrated to React - staff panel and delegate panel. This PR is to migrate the Delegate Panel to react. The staff panel will be migrated in a later PR.