revolution
revolution copied to clipboard
modxcms
Reorganize main menu
What does it do?
Changed the order of the main menu items. The items from the "Manage" section have moved to other logically related sections and a new "Access" item has been added and that's it.
What does it look like now:
All menu
Content
Extras
Account
The language switch is now in the system section. The language switch can be left in "Account" section, but it seems to me that this is more about the "System" than the "Account".
Access
All items related to users and access are moved to one section.
System
Related issue(s)/PR(s)
Discussed previously in https://github.com/modxcms/revolution/pull/14855
I re-opened a previously closed PR; the most useful thing about PR, it seems to me, is a new item for setting up access.
I like the access menu, but to me the language switcher is now too hidden. If I find myself on a site in a language that I’m not familiar with I look for a flag icon or a two-letter language designation somewhere obvious at the top level, not buried behind a menu in a language I can’t read or an icon that doesn’t scream language.
@rthrash Yes, it’s probably worth returning the language switch to the user section, as it is now. Although it’s unlikely that the language is switched often in the manager panel. After all, this is a manager panel, not a website.
That’s very fair. I typically switch it on the login page where it is fairly obvious.
As far as reviewing this goes, I have no strong opinions on this either way. I'm fine with both the current and the proposed menu structure.
Yes, I understand that the changes are not critical and can be left as they are. But I see 2 advantages in the new menu:
- New separate access menu item;
- More logical grouping of items. For example, even though I work with MODX every day, I won’t tell from memory where some items are located. With the new structure, I hope this will become more obvious.
I really do like this a lot and think it's a much more logical arrangement. I will get lost looking for the cache clear menu item the first few times, but I'll get used to it quickly. ;) Kudos @Ruslan-Aleev for your thoughtful work and the PR.
@Ruslan-Aleev This mostly seems like an improvement, however, there are a couple issues that came to mind when reviewing to incorporate into 3.1:
The first issue I didn't see addressed was related to ACLs. The default menu items have permissions associated with them and changing the parent/child relationships will potentially cause issues where users have limited permissions.
It's not clear (without testing) what might happen for those users with limited access (non Sudo) where access is being controlled to view submenu items via ACLs and the items change parents.
In addition, I'm not sure what impact adding a new item without corresponding permissions would have. How might someone prevent access to this new Access menu item?
The second issue is related to the ACLs but also to upgrades. Does the menu get updated on upgrade? What does it do to people's menu customization?
Generally, the changes aim to improve UX; that's a win. I do think it requires some investigation and testing related to the two issues I brough up. :)
@jaygilmore I didn’t quite understand the problem with the ACL, I didn’t create new policies, and I didn’t change them. Those what menu items were available to the user will remain so; It’s just that now some menu items have moved from system settings to the main menu.
The menu will not change during the update, it seems unnecessary to me, i.e. what the user has configured in his menu will remain. The new menu should only be installed during a clean installation, it seems to me.
Hey @Ruslan-Aleev,
I wasn't sure. If you're confident that this won't change things when upgrading and it's not going to change how existing users access things, that's fine. It just popped up as a thought when I was reviewing the PR. As I noted, I mostly see this as an improvement.
@jaygilmore Now I'm not 100% sure =) But logically, yes, we do not have an update script that affects the menu, for example like here. And I didn't create new policies, so there shouldn't be any problems.
@jaygilmore Now I'm not 100% sure =) But logically, yes, we do not have an update script that affects the menu, for example like here. And I didn't create new policies, so there shouldn't be any problems.
So these updates only apply to new installations? Existing ones won't get the updated menu layout?
So these updates only apply to new installations? Existing ones won't get the updated menu layout?
Yes, I think this is correct. I wouldn't change the menu at all when updating. Firstly, the structure changes too much, and secondly, the user could customize the menu for himself, but the update will reset this. This only makes sense for a new installation, I think.
@theboxer Thanks for your kind words.
I agree that the namespaces item is rarely used by users, but the purpose of the new menu is to group items based on their logic. I.e. the new menu shows the relationship between addons and namespaces, although this is not really necessary.
BUT if the guys @rthrash @Mark-H @jaygilmore @opengeek agree with your comment, then I will correct the PR.
@Ruslan-Aleev also one more thing, my head was going crazy trying to follow the menu structure and how it changed, this is not really related to your changes, but more to how the whole file is constructed. I adjusted it so the whole menu is defined as a single array with clear nesting of children and comments to separate everything out.
Looks something like this:
If you'd agree, I'd push the change to the PR (if it lets me), or send you the code so you can push it? (note, the menu structure is as proposed by you -- didn't touch namespaces, until hearing from others)
@theboxer
If you'd agree, I'd push the change to the PR (if it lets me), or send you the code so you can push it?
Yes, I'm for it!