Allowing authenticated users access to setup adds misleading message to config form

Before:

After:

Overall looks good.

There is one small oddity I encountered in a quick validation check.

If one sets the permission for a role (such as content editor) and then sets the Authenticated user to have the permission the content editor role permission is still present so it will not display the inheritance warning, however at the same time a site owner can not remove the permission from the role as it is inherited.

While the patch is actually technically more correct as currently written, should we consider performing the inheritance check first and then the hasPermission check for a more consistent UI?

If we were to do that however I imagine the Administrator role would show as inheriting, is that a better or worse UX than what is currently in the patch?

Leaving NR for additional feedback.

Since this is more technically accurate and brings up less questions merging as is..

Can be backported to 8.x-1.x manually (cherry-pick fails)

Thanks, sorry for being a bit slow addressing feedback! Happy to handle the mix of combined inherited + permission.

Wondering if the UI should become a tableselect like this instead as some of these descriptions are getting rather long. Perhaps this makes it easier to read? I left both on the screen for now to get your preference.

Wondering if the UI should become a tableselect like this instead as some of these descriptions are getting rather long. Perhaps this makes it easier to read?

That does appear to be easier to read and allow for a nice delineation between the role/description that is otherwise missing. I also like how the role is called out for both.

+1 from me (based on the screenshot) for the changes to be implemented.

Great thanks, I'll try to get this back to you in the next several days. Thanks!

Was simpler than I expected, seems its a true drop-in replacement element for standard checkboxes element:

NW per MR threads.

Thanks for the feedback, fixed both. Thanks!

Thank you for the work on this one!

Committed to 2.x dev, not sure how much of this we want to backport to 8.x-1.x, we still have MR!144 open, that is the 'first pass' solution, we could go with that, or we can look at if we should backport MR!145 to 8.x-1.x.

I noted in the thread that I wonder if we should backport with translated string changes. I will however note they are admin only and realistically MR!144 added new strings too so this wouldn't seem to be significantly different.

Would be a small UI layout change, we haven't really established a policy yet regarding how much UI can change, this does appear to be rather non-disruptive overall.

Flagging as need backport for discussions on what (if anything) makes it back to 8.x-1.x.