fix: Icon for toolbar button "user" - cms ui

[MoazMirza-13]

changed the user svg icon to settings svg icon. (ss below)

fix issue#https://github.com/plone/volto/issues/5119

[netlify[bot]]

✅ Deploy Preview for volto canceled.

Name	Link
🔨 Latest commit	d67fbb501d8e87ab4126bf6efc8469e2dceaeb27
🔍 Latest deploy log	https://app.netlify.com/sites/volto/deploys/653d92dd7ed2700008e50af6

[stevepiercy]

Sorry, it is not literal username but the authenticated user's username.

[MoazMirza-13]

Sorry, it is not literal username but the authenticated user's username.

@stevepiercy are we talking about this username ?

[stevepiercy]

Sorry, it is not literal username but the authenticated user's username.

@stevepiercy are we talking about this username ?

Yes.

[MoazMirza-13]

Sorry, it is not literal username but the authenticated user's username.

@stevepiercy are we talking about this username ?

Yes.

@stevepiercy got it now ! ps: i also changed the svg (now it is configuration svg). Hey, can you give this pr a hacktoberfest-accepted lable ? (video below)

video.webm

[stevepiercy]

Please ensure tests pass locally before pushing commits and requesting a code review. Thank you!

[MoazMirza-13]

@stevepiercy I am sorry to mention you like this but i tried various approaches to solve this but everytime I would get the same error in checks 'i18n' please look at my code and tell me what i did wrong. (as I'm new to open source contribution)

[stevepiercy]

You can read more about Volto contributing at Translations and Code quality for the failing CI checks. These must pass. It is best to do these locally before pushing commits.

[ichim-david]

@MoazMirza-13 the tests fail because of 2 reasons:

1. you need to run: yarn prettier:fix yarn lint:fix https://github.com/plone/volto/actions/runs/6633850565/job/18022304159#step:5:7 if you look to the left you will see 2 services that fail prettier and eslint https://github.com/plone/volto/blob/main/package.json#L48 here you will see all the scripts that you can run with yarn
2. You need to run yarn test to run jest tests https://github.com/plone/volto/actions/runs/6633850548/job/18022304459#step:5:3543 You will see that Jest is configured to save a snapshot which is tested against. Thus you need to update the test with the latest markup by pressing: u after the test fails. Once you commit the test snapshot update and the linting it will finally be green.

[stevepiercy]

@ichim-david I think Volto's documentation on what commands need to be run needs to be reviewed and updated.

• Code quality, which has links to the three pages:
• Linting
• Testing
• Acceptance testing

[ichim-david]

@ichim-david I think Volto's documentation on what commands need to be run needs to be reviewed and updated.

* [Code quality](https://6.docs.plone.org/volto/contributing/index.html#code-quality), which has links to the three pages:

* [Linting](https://6.docs.plone.org/volto/contributing/linting.html)

* [Testing](https://6.docs.plone.org/volto/contributing/testing.html)

* [Acceptance testing](https://6.docs.plone.org/volto/contributing/acceptance-tests.html)

Yup I looked at them after I saw your comments that @MoazMirza-13 should study but I noticed that they didn't document any of the ways to run the commands from the repo which are configured correctly using the proper rules.

This is why I wanted to give actual pointers on how to look and what to run.

There are so many moving parts with documentation now that I don't even know when to work on such a thing, we will talk about this later.

[stevepiercy]

I created a new issue https://github.com/plone/volto/issues/5341. It can be worked on at any time.

[MoazMirza-13]

@MoazMirza-13 the tests fail because of 2 reasons:

1. you need to run: yarn prettier:fix yarn lint:fix https://github.com/plone/volto/actions/runs/6633850565/job/18022304159#step:5:7 if you look to the left you will see 2 services that fail prettier and eslint https://github.com/plone/volto/blob/main/package.json#L48 here you will see all the scripts that you can run with yarn
2. You need to run yarn test to run jest tests https://github.com/plone/volto/actions/runs/6633850548/job/18022304459#step:5:3543 You will see that Jest is configured to save a snapshot which is tested against. Thus you need to update the test with the latest markup by pressing: u after the test fails. Once you commit the test snapshot update and the linting it will finally be green.

@ichim-david thanks for this yarn prettier:fix command, it worked

[davisagli]

@stevepiercy @sneridagh Has the Volto team discussed this? I understand the arguments for why the current icon/menu is confusing, but I'm not clear on whether there is consensus on how to change it, and if there is, I think it should wait for a major version (we shouldn't just change a major UI element in a patch release).

[stevepiercy]

@davisagli I don't think the Volto Team discussed it. I put it on the agenda for our next meeting.

I discussed it with the Classic UI team and here is the result of that discussion: https://github.com/plone/volto/issues/5119#issuecomment-1727737649.

[ichim-david]

@stevepiercy @sneridagh Has the Volto team discussed this? I understand the arguments for why the current icon/menu is confusing, but I'm not clear on whether there is consensus on how to change it, and if there is, I think it should wait for a major version (we shouldn't just change a major UI element in a patch release).

@davisagli I don't think that anyone mentioned that this will be merged as part of a patch release.

I think the fact that this issue was solved in a way is a good thing, then it's up to Victor and / or the Volto Team to discuss when this fix should be merged.

Ex one of my pull requests regarding some a11y changes Victor though it's a breaking change that will go in 18 only https://github.com/plone/volto/pull/5294 that doesn't mean I shouldn't have worked on it thinking that it would not make It to 17 but simply that I wanted to fix something and then it's up to you guys to decide when it's fitting to be merged.

[sneridagh]

@davisagli @stevepiercy @ichim-david Indeed, this has to be discussed and approved by the Volto Team.

/cc @plone/volto-team

[sneridagh]

Some considerations, if we make this change, it should be done in Classic as well under the same premises.

/cc @plone/classicui-team

FTR, that icon is not the one that Quanta defines, it should be the "cog":

https://www.figma.com/file/YpaRSKjFcFaAkVYnTSVGuz/Volto-UI-%E2%80%93-Quanta?type=design&node-id=1812-1140&mode=design&t=QKPrqnfZ3DRdox43-4

[avoinea]

In 90% of the cases the users don't have the Manager / Site Administrator role. Thus, the menu will not contain the Site Setup or any admin settings. In this case, the user icon is more suitable, as it is only about the user profile, preferences and logout.

[avoinea]

Also replacing the user <Name> with Settings seems a major drawback to me.

[ichim-david]

@MoazMirza-13 I am going to close this pull request. Based on the discussions from all of the core developers it seems that it's simply better to keep the current behavior than to change the icon and text due to the permissions of users. Thank you for your attempt to fix this issue, your code was good enough to merge but upon further review of the problem it's better to keep current behavior.