[UX] Layout overview page: Add indicator for menu-level visibility conditions

[docwilmot]

On layout list table we've added VCs as a column for layouts, but menus can have VCs too. An administrator on a complex site might end up really puzzled about 403/404 errors if he doesnt realise menu VCs have been set.

[klonos]

I don't see any visibility conditions for menus or menu items. Are you referring to menu blocks perhaps? ...or is it something I'm missing?

[docwilmot]

I don't see any visibility conditions for menus or menu items

You have to add a custom layout first, then click "menu settings"

[docwilmot]

Five year delay! Took a look at this today, and here is the result. I think this should be done because the fact that paths can have VCs is completely hidden and could really confuse a person. Note for example dashboard layout has a VC, but who knew the path had a VC too? Is the page too busy now though?

[argiepiano]

Who knew there were VC for paths! In fact you have to dig deep into the UI to find them - they are not set when you create a layout. This, in itself, is already problematic for me, but I can't think of a way to change this.

As for your screenshot: This is a UX question, and it's not my "cup of tea", but to me, the gray separators with the path should not follow the column (I think this is part of what makes the screenshot confusing). They should clearly look like separators.

Plus, there is another settings for paths that'd be nice to show: the "type" (i.e. "No menu entry", "Normal menu entry" etc).

So, perhaps more of a "summary" approach, rather than using the column?

[docwilmot]

Good suggestion. If there are multiple VCs it would still be a bit crowded there though. Wonder if a "more" link like on the modules page would work here.

[bugfolder]

Wonder if a "more" link like on the modules page would work here.

So do I. https://github.com/backdrop/backdrop-issues/issues/5090. @klonos 😉

[klonos]

So do I.

I know 🙂 ...I have some vacation time towards the end of July and end of August, and this is on my (long) list of to-dos 👍🏼

[bugfolder]

Since I'm being such a nag, I hasten to add that I'll be happy and eager to test, etc.

[klonos]

This one is puzzling indeed 🤔

• First, I agree that we should be exposing that information somehow 👍🏼

• From a UX point of view, it makes sense to add the visibility condition to the existing column with the same name.

• On the other hand, I can see @argiepiano's concern ...the suggestion does not account for multiple visibility conditions though, which should be shown in a list (for readability/scannability). Having multiple visibility conditions shown sequentially would make it problematic.

• If we were to add that info in a "more" link, it would take another click to reveal, which is the same as clicking the "menu settings" button (albeit you'd remain on the same page, which is a 👍🏼)

• Since we are talking about changes in the table in order to accomodate this info, it has always bothered me that the preview icons for the layout templates are in the same column as the layout name and machine name, yet there's a separate "Template" column with the name of the template as a link to the settings page:

  • Visually, as well as from a common UX pattern point of view, it makes sense for the icon to be to the left-most side of the table 👍🏼
  • Splitting relevant info across two separate columns though 👎🏼

  I've considered merging the template link with the rest of the layout/template details in the first column (which would leave room for things like the menu type), however that would make the first column way too "busy" then, and given that that column holds the most important info in the table, that's a 👎🏼

Anyway, I am suspecting that it's be hard to come up with something that's intuitive and readable, yet at the same time does not add to the complexity and "noise" in this table. However, can we please try the following?:

• Add the menu type as a "machine name" to the path name, mimicking the output of theme_label_machine_name() (but not using it directly) ? The visual result should be smaller, gray text, on a second line, no brackets, with the prefix Menu type:.
• Keep the visibility conditions in the respective table column, as in @docwilmot's screenshot/mockup.

I admit that the above would still not be ideal - separators should be running across the length of the table. I'll do some more thinking and research and see if I come across anything that useful in this situation.

PS: I've tagged this issue with UX/design and am "summoning" the @wesruv for thoughts/ideas 😅

[stpaultim]

I really like the idea of exposing this. I'm not sure that I would have ever noticed that this was possible. I am a bit worried about this form getting too cluttered, as others have already expressed. But, don't have much new to offer in terms of ideas.

I look forward to testing the PR! ;-)

[docwilmot]

PR up for consideration. Used details element to hide things.

[argiepiano]

@docwilmot thanks for putting this together! I think this is much better than what we have (or don't have) currently. Here are a couple of thoughts (keep in mind that my UI chops == FALSE).

• Since "Menu Settings" now directly affect the "Path info", I wonder if it may be clear to call the button "Path settings" or the info "Menu info"? I like the first option better. Technically that button does more than Menu Settings. It does Path Settings, as you can set conditions
• Wondering if the info could extend beyond the first column? It's kind of strange to see the line break there, after "Normal menu \n entry"

[docwilmot]

Wondering if the info could extend beyond the first column? It's kind of strange to see the line break there, after "Normal menu \n entry"

OK made it colspan 3

Since "Menu Settings" now directly affect the "Path info", I wonder if it may be clear to call the button "Path settings" or the info "Menu info"? I like the first option better. Technically that button does more than Menu Settings. It does Path Settings, as you can set conditions

Makes sense, but would need some group decision-making. We can wait for some more input on that.

[argiepiano]

I like it.

[olafgrabienski]

Since "Menu Settings" now directly affect the "Path info", I wonder if it may be clear to call the button "Path settings" or the info "Menu info"? I like the first option better. Technically that button does more than Menu Settings. It does Path Settings, as you can set conditions

In theory, "Info" and "Settings" should be enough, but I guess "Settings" could be confused with the dropbutton link "Configure Layout" then. So, maybe just "Info" for the more link (to not repeat the term "Path" directly after the "Path" label), but "Path settings" for the Operations column? Mockup:

[olafgrabienski]

@docwilmot After creating a simple node layout (node/%) withour further conditions in the PR sandbox, I got some warnings (see below). Also "Menu" and "Menu item type" are missing from the Info, and there is no "Menu settings" button in the Operations column.

Warnings:

• Attempt to read property "conditions" on false in theme_layout_path_info() (line 127 of core/modules/layout/layout.theme.inc).
• Attempt to read property "menu" on false in theme_layout_path_info() (line 127 of core/modules/layout/layout.theme.inc).
• Trying to access array offset on null in theme_layout_path_info() (line 127 of core/modules/layout/layout.theme.inc).
• Attempt to read property "menu" on false in theme_layout_path_info() (line 133 of core/modules/layout/layout.theme.inc).
• Trying to access array offset on null in theme_layout_path_info() (line 133 of core/modules/layout/layout.theme.inc).
• Undefined array key "" in theme_layout_path_info() (line 133 of core/modules/layout/layout.theme.inc).
• Attempt to read property "menu" on false in theme_layout_path_info() (line 134 of core/modules/layout/layout.theme.inc).
• Trying to access array offset on null in theme_layout_path_info() (line 134 of core/modules/layout/layout.theme.inc).
• Undefined array key "" in theme_layout_path_info() (line 134 of core/modules/layout/layout.theme.inc).
• Attempt to read property "conditions" on false in theme_layout_path_info() (line 136 of core/modules/layout/layout.theme.inc).

[docwilmot]

Ah, missed that, should be fixable shortly.

[docwilmot]

COde issues mostly resolved now, only some PHPCS fixes to go.

[stpaultim]

To give good feedback on the UX for this, I need to better understand how it's used.

I assumed that the idea was that I might have a path (example: admin/dashboard)

Then I might have two different layouts for this path, each with their own visibility condition. Example one dashboard for admins and one for editors. I assumed that the menu visibility condition would override the layout visiblity conditions.

However, I created the two layouts, one for admins and one for editors. I then ONLY gave menu/path access to admins. However, in this case, editors were still able to access their own special dashboards, despite not having permission to that menu/path.

So either there is a bug OR I'm not understanding menu visibility conditions for layouts.

Can anyone provide a practical example of how one might use menu level visibility consitions?

[docwilmot]

This seemed to be working for me.

• I created a path with two layouts
• I added the VC "user has role: New Role" to the menu path
• Tested that no user without that role could access that path (coincidentally, neither could admin, which should be a bug)
• I added Editor role to another user just to be sure Editors dont have magic powers, but no, Editor couldnt access the path either
• I added a "user has role: Editor" VC to one of the layouts under that path, and tested that an Editor still couldnt access it
• I created a new layout under admin/dashboard and added VC "user has role: New Role" (in addition to the User has "Access Dashboard" VC that already exists.
• Only users with New Role could access any dashboards from then, regardless

Perhaps I need a bit more detail on how you tested @stpaultim

[stpaultim]

@docwilmot

My mistake. I created a new Dashboard and was experimenting with different roles accessing different versions of the Dashboard based upon visibility conditions. However, this seems to have been a bad choice, because the Dashboard has it's own access permission that may trump the visibility conditions for the menu or layout. Both roles had "Access Dashboard" permission, so my changes to visibility conditions had no effect.

I think that makes sense.

When I tried the same experiment with a new path and a completely new layout, it worked as expected and as you described.

[stpaultim]

@docwilmot - To be honest, this idea that the path for a layout might have it's own visibility conditions was a bit confusing for me. Given what I now understand much better, it seems important that path visibility conditions be in the same column as those for the layouts - so that they don't seem less important.

This might mean going back to the original idea of having them visilble all the time.

OR still hidden in the "Path Info" carrat, but when visible, visible in the same column as the other visibility conditions.

I actually think I like this second option better. Hidden, but when the info appears it's structured to show the VC in the same column as the other VCs.

I am also thinking about some tweaks to the help text under "How layouts work", but that will be a new issue.

[docwilmot]

@stpaultim I find that if I click a details element I'd want the info thats revealed to be right under the details link, not so far away. I can imagine someone might miss seeing the VCs on a wide screen if theyre over in a diffrent column. not sure I like this.

Or maybe give VCs their own details element? But that makes the table really busy to me, and Layouts are confusing enough it seems.

@wesruv any design advice?

[stpaultim]

I can imagine someone might miss seeing the VCs on a wide screen if theyre over in a diffrent column. not sure I like this.

I see that point. But, I could argue the same that people might miss the VC's if they are not in the column labelled Visibility Conditions.

:-)

I don't see this as a blocker. Either way is a step forward in exposing "feature." I'm guessing that I am not the only one that didn't know that visiblity conditions are available for paths. Anything that helps expose them, might help folks better leverage the layout system

The best solution for exposing this feature MIGHT be better help text in the How Layouts Work section of the page. This is a different issue, I think.

Looking forward to hearing what others think?

[quicksketch]

I read all the comments above to get a handle on the approaches and I tried out the current sandbox. Thank you @docwilmot for patiently trying so many things!

My opinion: we might be getting more fancy than is really needed. I would prefer we just print the visibility conditions for a path all the time in the "Visibility Conditions" column, as @docwilmot originally suggested. I also don't think we need the "Path info" details element at all. Whether the page is a normal menu item or a tab is not so important it needs to be on the overview page.

[stpaultim]

As someone that originally expressed concerns over how cluttered this would make the UI, I think I've come around to liking that intial simple view, now that I understand the issue better and have seen the alternatives.

[docwilmot]

Did a new PR instead of fixing this one.

[stpaultim]

@docwilmot Thanks for trying those other things out. But, I like the simplicty of this last PR. I think it is the right solution for displaying menu-level visibility conditions.

I think we might also need some more help text regarding paths, but I think that can and should be left as a follow-up issue. Simply making them more visible is a good first step.

Here is a screenshot:

This PR exposes and shows the info inside red boxes. This info was not available before, except on Menu Settings page.

[quicksketch]

https://github.com/backdrop/backdrop/pull/4857 looks good to me. There are a few code style failures but nothing caused specifically by this PR -- in fact it fixes dozens of code style issues. I don't want to merge this without further agreement however. @olafgrabienski, @argiepiano, @bugfolder, @klonos if you could post your thoughts that would be appreciated.

[argiepiano]

This looks good to me - the changes have minimal impact on the UI, and the PR does what the OP says.

I think the availability of two different visibility conditions (one for path, another for page) may confuse people (esp. which of those takes precedence, and why does the path visibility exist if one could simply use a VC in the page itself - this becomes clearer if you keep in mind that you could have more than one page that use the same path, and the path VC will then be like a global VC for all those pages). Adding some text to the details like @stpaultim suggested in https://github.com/backdrop/backdrop-issues/issues/6699 may help.