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
Fix and enhance usergroup permissions handling and display #16469
base: 3.x
Are you sure you want to change the base?
Conversation
@muzzwood - Hey - wanted to see if you'd have time to review this fix, since it's related to one of the issues you posted. |
b611d88
to
e4dc4a2
Compare
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Jim Graham.
|
71da7e1
to
4100673
Compare
@GulomovCreative @Ibochkarev @muzzwood -- If any of you guys have time to test/review this, that'd be great! |
@smg6511 great UX improvement … thanks for submitting the PR and looking forward to seeing this in 3.1. |
What does it do?
Among a number of refactoring moves, the solution submitted here provides a new and centralized method to update the “Permissions in Selected Policy” component found in the various Create and Update windows within the ACLs area. Also:
Special Note
In the editing windows, I changed the Context combo’s displayField to
name
instead ofkey
(context_key
). Even as a developer, I've always found showing the key cumbersome — to me it's much more natural to associate with the name of any object rather than its id/key. If reviewers and code owners agree, I'd also implement that in the grids (which currently still display the key).Why is it needed?
As mentioned in #16386, permissions lists (in windows) were not updating correctly. Also:
Figure 1 — Before fix, missing perms list
Figure 2 — Window before and after, showing css update
Figure 3 — Grid before and after, showing css update
Figure 4 — Video clip showing correct permissions display when opening and after closing and re-opening editing windows
https://github.com/modxcms/revolution/assets/689075/f008cd55-97fb-432a-aa7a-d2bf8c2b28df
How to test
Related issue(s)/PR(s)
Resolves #16386