feat: added custom condition to the aria-label user button #6346
Conversation
✅ Deploy Preview for plone-components canceled.
|
Wagner3UB
requested review from
pnicolli,
JeffersonBledsoe,
ichim-david and
SaraBianchi
| @@ -0,0 +1 @@ | |||
| - added custom condition to the aria-label user button @Wagner3UB | |||
There was a problem hiding this comment.
@Wagner3UB Could you be clearer what the condition is for in the changelog?
There was a problem hiding this comment.
| - added custom condition to the aria-label user button @Wagner3UB | |
| - On the user button, whether the authenticated user is an administrator or not, change the `aria-label` attribute accordingly. @Wagner3UB |
There was a problem hiding this comment.
Also I think this needs to be renamed to 6346.breaking.
There was a problem hiding this comment.
What is this PR breaking? I don't see it right now but maybe I missed it.
There was a problem hiding this comment.
It changes the aria-label from "Personal tools" to either "Site and user settings" or "User settings".
| const { userId, getUser } = this.props; | ||
| getUser(userId); |
There was a problem hiding this comment.
@Wagner3UB Did you test to see what would happen if you logged in as one user who does have permission, logged out, then logged in as a different user who doesn't have permission?
I would assume the state gets updated elsewhere and handled by the rendering of this component, but it would be a good thing to check.
| userHasRoles(this.props.user, [ | ||
| 'Site Administrator', | ||
| 'Manager', | ||
| ]) | ||
| ? this.props.intl.formatMessage( | ||
| messages.adminUserlTools, | ||
| ) | ||
| : this.props.intl.formatMessage(messages.userTools) |
There was a problem hiding this comment.
Can you make the message a variable so it is easier to keep in-sync with the above button?
| @@ -67,6 +69,14 @@ const messages = defineMessages({ | |||
| id: 'Personal tools', | |||
| defaultMessage: 'Personal tools', | |||
| }, | |||
| adminUserlTools: { | |||
| id: 'Site tools and user settings', | |||
There was a problem hiding this comment.
make the id same as default message
There was a problem hiding this comment.
Should we also remove personalTools entirely?
| @@ -0,0 +1 @@ | |||
| - added custom condition to the aria-label user button @Wagner3UB | |||
There was a problem hiding this comment.
| - added custom condition to the aria-label user button @Wagner3UB | |
| - On the user button, whether the authenticated user is an administrator or not, change the `aria-label` attribute accordingly. @Wagner3UB |
| @@ -0,0 +1 @@ | |||
| - added custom condition to the aria-label user button @Wagner3UB | |||
There was a problem hiding this comment.
Also I think this needs to be renamed to 6346.breaking.
| @@ -67,6 +69,14 @@ const messages = defineMessages({ | |||
| id: 'Personal tools', | |||
| defaultMessage: 'Personal tools', | |||
| }, | |||
| adminUserlTools: { | |||
| id: 'Site tools and user settings', | |||
There was a problem hiding this comment.
Should we also remove personalTools entirely?
Added a condition to set the aria-label to differentiate the "regular users" from the admin users.
The condition is the same one used inside the 'PersonalTools' component, which opens after the user button is clicked.
Issue with discussion:
#5119