- Add whitespace between modules for increased readability.
- Add role labels every third module header. Increases the ability to map checkboxes to roles without being too busy.
- Use th cells instead of td for module headers.
- Removes the 'click a role header to view only that role' feature. It breaks the expectation of what should happen when clicking a table header (it should sort) and the need is reduced by increased labeling.

CommentFileSizeAuthor
#3 user_68.patch7.15 KBdrumm
#1 user_67.patch7.14 KBdrumm
screenshot_27.png37.26 KBdrumm

Comments

drumm’s picture

drumm
he/him
Location NY, US
commented
StatusFileSize
new user_67.patch7.14 KB

And a patch.

drumm’s picture

drumm
he/him
Location NY, US
commented
Title: More labels on permissions page » More labels on Access control page
drumm’s picture

drumm
he/him
Location NY, US
commented
StatusFileSize
new user_68.patch7.15 KB

I made the roles in the module headers slightly darker to make the contrast just high enough to be compliant to W3C accessibility guidelines.

mcurry’s picture

mcurry commented

I like this. Question: why not repeat the header on every module?

(I'd love to figure out how to keep the header row visible at the top when I scroll.. I've seen that done around the web, but I think the HTML markup is probably too ugly/fragile.)

dries’s picture

dries commented

- Repeat the roles on every module. It feels 'buggy' not to.

- What is strange about this table is that the headers have a white background but that the actual content has a grey background. It feels 'inverse' to me.

- I'd sprinkle some code comments around.

mcurry’s picture

mcurry commented

I like the feature that allows the user to click the role name to show only that role. In fact, I didn't know that feature existed, until I saw this patch. That's what I get for not digging deeper!

I'd never expect clicking on the role name to sort by that column, since the column is a bunch of checkboxes...

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Needs work

When I get a chance to rewrite this and add the role links back in and other suggestions. Really its what we currently have with the 'Filter by: [ ..... |v]' convention on other admin pages now. A separtate patch (anybody else want to do that one?) can handle that.

dmitrig01’s picture

dmitrig01 commented
Version: x.y.z » 6.x-dev
webernet’s picture

webernet commented
Status: Needs work » Closed (duplicate)

The header row now scrolls with the page.