search

backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 40 forks source link

[UX] Add help text on the Layout listing page to explain order and grouping. #4868

Closed klonos closed 2 years ago

klonos commented 3 years ago

This is related to #4841 and was brought up during the Dec. 31, 2020 weekly UX meeting, https://youtu.be/Fc-oQ55LQi0?t=2190 (~37min into the call)

@philsward (referring to the layout listing page) explained:

...just coming and looking at it, I feel that everything is jumbled together ...I don't know what I'm looking at. Nothing is organized, and that was kinda my big beaf with it.

@jenlampton said:

...everything is organized, but it's not clear to anyone looking at the page that it is. ...it's confusing.

Then @stpaultim asked a question about the organization of things:

...I've always understood that the higher up a layout is on the list, the more priority it has.

...then @jenlampton went on, to explain how the grouping and priority works on the layout listing page.

@cellear:

I just learned 30sec ago that the order in this page matters. I had no idea.

@stpaultim:

It doesn't say that anywhere. ...the only reason I think I know that is I think I've heard @jenlampton say it, but I might have misunderstood.

@jenlampton

...and if you have 2 things that can be ordered, a button shows up that says "Reorder". But if you don't ever have them, you would never know that. ...if you never create anything that needs an order, you'll never even know that there are things that can have an order.

This is what @jenlampton was referring to:

Screen Shot 2021-01-12 at 6 04 08 pm

Proposed solution:

Screen Shot 2022-01-03 at 2 37 35 PM

Translation update

New strings:

klonos commented 3 years ago

PS: @stpaultim mentioned that when creating a node-specific layout (like node/3), he noticed that it is listed after the "catch-all" node/% layout, but node/3 takes precedence over node/%. This will need testing/confirmation, to clarify if it is expected behavior or a bug. If a bug, we'll need a follow-up issue for it.

klonos commented 3 years ago

Suggested help text (broken down to short sentences, in the form of bullet points):

  • The various paths that layouts handle are listed here alphabetically.
  • Each path is used as a group, to hold all layouts that may possibly apply to it (layouts are grouped within the path they may handle).
  • To determine which layout is to be used for a page, the path of the page is matched against the ones in this table.
  • Paths without a layout explicitly defined for them are not listed here. In that case, one of the Default layouts will be used (listed at the bottom of the table).
  • Each path listed here may have either one or multiple layouts defined within its group:
    • If any path has exactly one layout defined, then that layout will be used (unless visibility conditions rule it out).
    • If any path has multiple layouts defined, then:
    • Starting from top to bottom, all layouts within the path group are checked for matching visibility conditions.
    • The first layout within the path group that matches all conditions will be the one that gets used.
    • To affect priority, you may reorder any layout within the same path group.
    • If no matching layout is found, it will fall back to one of the Default layouts.

For more information and details, see the [online documentation]().

klonos commented 3 years ago

Although people new to layouts may appreciate this help text, adding such a big chunk of text in that already busy page may be annoying to people that already know how layouts work. Also, it may become less useful after people have learned how things work.

For some time, I have liked how Wordpress handles "inline" help, and wanted to propose that we do something similar to this:

wp-help-expand-collapse

...well, I think that this issue here is the perfect opportunity to test and consider a new UI pattern for such situations. So here's a PoC PR, which uses a collapsible, and collapsed-by-default fieldset, to mimic the WP expanding/collapsing help widget: https://github.com/backdrop/backdrop/pull/3492

backdrop-help-expand-collapse

philsward commented 3 years ago

The suggestion looks good and I like the idea of site wide help. I think it's worth exploring.

I keep thinking I will get around to a mock-up of how I think the GUI could be easier to understand but haven't had the time to dedicate to it. 😕

stpaultim commented 3 years ago

I love this idea of contextual help. My only concern is that we have many other pages that could benefit from this kind of help text and is this the pattern we want to adopt or is there a better one. We've also discussed tooltip solutions.

https://github.com/backdrop/backdrop-issues/issues/3707

One of the blockers for any kind of "help" module in core seems to be the lack of content for it and uncertainly that such content would ever get created if the module existed. Chicken vs Egg dilemma.

Maybe we could adopt some UI/Design scheme like this as a placeholder for a larger "help" module in the future, with the idea that anything put into this kind of accordion styled help block would be moved to any new help module in the future.

Just a thought.

klonos commented 3 years ago

We've also discussed tooltip solutions.

I know, but the solution I've used in this PoC is basically re-using something that exists in core already (simple fieldsets with text of '#type' => 'help'), whereas something like #3707 would require much more work (and most likely introducing another 3rd party library in core).

Maybe we could adopt some UI/Design scheme like this as a placeholder for a larger "help" module in the future, with the idea that anything put into this kind of accordion styled help block would be moved to any new help module in the future.

Yeah, the help module was removed from Backdrop core. See https://github.com/backdrop/backdrop-issues/issues/75#issuecomment-43466880

Yep, it's expected that you'll add help on the pages directly, such as $form['help']['#markup'] = $help.

...which is what I did here ...only within a collapsible fieldset 😉

bugfolder commented 3 years ago

Looking at both the expandable-help (described in this issue) and the tooltip suggestion (in #3707), while both would be big improvements, I favor the tooltip approach for two reasons.

  1. It's smaller in the collapsed state. The expanding-help fieldset takes up a full "row" of space, even when collapsed. The tooltip is just another character, and so it can be tucked into places that a full row would be problematic. That allows tooltip-help to be more fine-grained; you can put lots of different tooltips in a form (or even on individual items in a table), so that individual tooltips can be more precisely targeted.

  2. This is just a personal aesthetic, but having a popup overlay appear (with a tooltip) seems a little bit friendlier than having something expand and push page content down and out of the way. I recognize that having the extra information push down the existing page content gives the user the option to scroll back and forth between the help text and what it applies to, while an overlap covers up what it applies to, so I see arguments on both sides. On interfaces that support hovering (e.g., desktop, vs mobile), though, it would be nice to have the help text appear on hover (or let that be a theming option).

@stpaultim pointed out that an issue would be the lack of existing content, which is true. If tooltips started appearing widely in core, though, that would be an incentive for contribs to adopt it. And if adding a tooltip was as simple as adding the '#tooltip' => ... to form elements and render arrays, creating tooltips in both core and contrib would be easy ways for the community to contribute. One of the intellectual barriers to submitting one's first PR is the concern "does my solution have a bunch of side effects that I don't recognize or run afoul of practices I don't know about yet?" Creating tooltip PRs would be "good first issues".

bugfolder commented 3 years ago

Based on PR https://github.com/backdrop/backdrop/pull/3492, I've submitted PR https://github.com/backdrop/backdrop/pull/3754 with these differences:

jenlampton commented 3 years ago

The link to the "online documentation" previously just went to a listing of menu items. I've replaced it with a link to the "Layouts and Templates" page in the User Guide on docs.borg. However, that's a fragile thing to do, since that page's URL might change in the future, so I solicit alternative suggestions.

That's the right thing to do. If the URL changes in the future a redirect will be created.

stpaultim commented 2 years ago

This needs ad advocate to be added to the 1.21 milestone. However, I think that this would be considered a UX improvement and could be added to the bugfix milestone. Not sure it matters, because I think we should be able to get this into either.

I like it and would mark it works for me, but I think it would be better to get a little more feedback on the appearance and content first.

I still have questions about how the way we are handling this help text relates to how we are or will handle it elsewhere in core. I will bring up these questions at the Design/UX meeting tomorrow.

bugfolder commented 2 years ago

Discussion at the 12/2 design meeting: the word "Help" as the title might be less useful than something more specific: "How Layouts Work", or something like that. Suggestions solicited.

klonos commented 2 years ago

I still have questions about how the way we are handling this help text relates to how we are or will handle it elsewhere in core.

I also don't want to hold this back, so I wouldn't worry too much about styling and consistency of these help elements - it can be a follow-up issue (perhaps return of the help module in core? 🤷🏼 ).

Re the concerns that were surfaced during the recent design meeting around how "prominent" this help element is, and how it may "draw attention" away from the main actions in the page, here's a quick mockup of how that would look as a "help bar", right bellow the admin bar (with reduced font size + topic subject added next to the word "Help"):

image

That would mimic the WP paradigm a bit closer, and get this help "out of the way".

indigoxela commented 2 years ago

... get this help "out of the way".

I'm not sure if I'd realize, that this far away collapsed thing directly beneath the admin bar is the help for layouts. Things above the tabs are usually something unrelated. At least, for me.

IMO, if that details element is collapsed it's not too prominent. One thing's true: only the word "Help" isn't enough for the summary. "Help: How layouts work" seems better.

Maybe in Backdrop we don't need lots of these extra help texts, but for layouts this additional information is for sure useful. For people coming from Drupal and people coming from any other CMS. Many things in Backdrop are sort of self-explanatory - layouts are not (views aren't, either).

What's possibly not ideal, is the current formatting of the help text - I find these nested lists hard to read.

Just my 2c. :wink:

bugfolder commented 2 years ago

Per comments above, I've made two changes to the current PR:

Results in the Tugboat build; looks like this:

sample

jenlampton commented 2 years ago

I love, love, love this!

Suggestions for text changes (mostly shortening / simplifying for clarity):

I tried to remove any mention of "table" or "group" because how the things are organized on the page isn't as important as what that organization means, but if you think it was important to mention I could be convinced otherwise :)

bugfolder commented 2 years ago

because there is nothing in Backdrop called a "path alias", it was renamed to "URL alias".

There is a "path alias" here: https://docs.backdropcms.org/api/backdrop/core%21includes%21path.inc/function/backdrop_get_normal_path/1

This page uses both "URL alias" and "path alias": https://docs.backdropcms.org/api/backdrop/core%21includes%21path.inc/1

A search through core code turns up 61 "path alias" but 187 "URL alias". So I can see the latter is preferred, but I guess we got some updating to do.

Up until now, I'd been using "system path" and "normal path" to mean different things: node/%/edit is a system path, but node/1/edit is a normal path (what gets returned by backdrop_get_normal_path(). But on further examination of docs and functions, I see "system path" being used for "normal paths", so I guess I'm going to need to rewrite my brain-config.

So what kind of path would we call node/%/edit when we're trying to make distinctions between the different types of path? "Menu router path" seems a bit unwieldy and obscure for our intended audience.

bugfolder commented 2 years ago

Anyhow...I've made all those changes (with a few minor tweaks), and also changed the details class to description (so it will be styled by the CSS added in https://github.com/backdrop/backdrop-issues/issues/5399). Here's what it looks like (with the older styling).

From Clipboard

stpaultim commented 2 years ago

Works for me!

But, given all the text and discussion - I think I'd like one more person to endorse this version before marking it as such.

indigoxela commented 2 years ago

It also works for me, so marking it as such.

klonos commented 2 years ago

This is a task, so it could in theory get in with any bug fix release, but since the milestone label was set for the next minor, I've set the milestone to 1.21.0. If we happen to miss that window for whatever reason, it should get in with any next 1.21.x release I think.

I've reviewed the code, and I'm happy with it, so RTBC 👍🏼

We can tweak the wording further as we see fit, and as more PRs that affect the layout listing page and/or the way layouts work get merged.

klonos commented 2 years ago

...re wording, I've left a couple of suggestions, but I'd prefer it if we discussed it in person, to avoid back and forth changes. It will also help if another person takes a look at the text and chimes in. @jenlampton ?

jenlampton commented 2 years ago

I think the PR looks really good and is already a huge improvement over what we have now. PR is RTBC from me -- and any further text changes could be done post-merge if necessary.

That said, I'm going to file a PR against @bugfolder's PR (mostly so I can see what it looks like with my suggestions).

jenlampton commented 2 years ago

I think I have something I prefer. Feedback wanted! Here's my PR

Here's where I landed:

Screen Shot 2022-01-02 at 8 24 23 PM

Changes include:

bugfolder commented 2 years ago

Agreed, this is great. @jenlampton's PR merged into https://github.com/backdrop/backdrop/pull/3754.

docwilmot commented 2 years ago

I have a few comments and recommendations:

I think we should state first that paths lead to a layout, then introduce VCs after this is clear, otherwise multiple new concepts in one bullet point is too much. The path examples re system vs alias etc are more likely to confuse IMO. We know what those are but to a newbie, there is no difference between node/1, about and node/%; theyre all web addresses. If the user wants to clarify this, they should research the more extensive document.

bugfolder commented 2 years ago

Although...I just noticed that @jenlampton is referring to what layouts accept as "system paths". I think what they want are called "menu router paths" in the few places they are explicitly named, like https://docs.backdropcms.org/api/backdrop/core%21modules%21field_ui%21field_ui.module/1).

There are many places in the documentation where the term "system path" is used as a synonym for "normal path", like:

On this page, though, "system path" is indeed used for layout paths:

It seems like there's more usages of "system path" as a synonym for "normal path".

On the other hand, for purposes of user-friendly documentation, "system path" is a nicer name than "menu router path", so there is some logic to adopting "system path" for that usage. But that would mean that a bunch of places in the documentation would need to be updated.

So: should "system path" be the official public name for menu router paths like node/%?

docwilmot commented 2 years ago

I think system path is a proper usage, but my comment above was more what would be understandable rather than what is accurate.

stpaultim commented 2 years ago

I've reviewed the latest text and I think that this will be very helpful to users.

jenlampton commented 2 years ago

Although...I just noticed that @jenlampton is referring to what layouts accept as "system paths". I think what they want are called "menu router paths"

I'd be fine with changing that laguage to "Menu router paths", we could even add something like As defined in hook_menu if that would be useful.

jenlampton commented 2 years ago

Thoughts on @docwilmot's recommendations :)

I think we should state first that paths lead to a layout, then introduce VCs after this is clear, otherwise multiple new concepts in one bullet point is too much.

My goal was to include all of the new concepts in a concise paragraph that painted a complete picture, before we started with the bullets. Keeping to that model, we could do something like the following for the first sentence.

As a page is rendered, a Layout is selected. The Path for the page is compared to those listed here...

Question: Do people prefer all bullets to a paragraph before the bullets? I went back and forth on this, and ended up deciding on a paragraph first so that people would get the whole picture of "How layouts work" before the details of "what's on the page", and "how to use it" (things in the bullets). I don't know if this separation makes sense to everyone else :)

As a page is rendered, its Path is compared to those listed here to determine if...

I think people need to understand that a layout is always selected for a page. The "to determine if" makes it sound like it's possible for no layout to be selected. we could try "to determine which" (but I still feel that the current paragraph text, with shorter sentences, is more clear)

...a particular layout should be assigned to that Path.

I'm not comfortable with "assigned" here -- is this intended to mean "available" or "selected"? (see below)

Each Path listed below may have one or more available layouts assigned.

I don't like "assigned" here, that sounds like they have already been selected (perhaps because of above usage?). What about just switching "one or more available layouts" to "one or more layouts available"?

Visibility conditions will also be evaluated to determine if the user is allowed to access this layout.

This isn't quite right, because visibility conditions are not usually about user access (and, we don't want to use the word "User" anyway). "Visibility conditions will also be evaluated to determine if a layout should be selected" is more accurate.

to a newbie, there is no difference between node/1, about and node/%; theyre all web addresses. If the user wants to clarify this, they should research the more extensive document.

I think this is even more reason that we need to provide examples. I see so many people entering node/1 into layouts when what they meant was node/%. If show them right here that node/1 is not what they want, it will save them a lot of headaches.

I'm going to try a PR to fix the first sentence, and switch "System path" to "Menu router path", and try to add in the word "selected" for clarification.

jenlampton commented 2 years ago

Okay, I think this is an improvement:

Screen Shot 2022-01-03 at 2 37 35 PM

Changes include:

bugfolder commented 2 years ago

LGTM.👍

stpaultim commented 2 years ago

I'd be fine with changing that laguage to "Menu router paths", we could even add something like As defined in hook_menu if that would be useful.

I have to say that "menu router paths" actually does not mean anything to me, although I know it should. System path was (marginally) easier for me to understand. HOWEVER, I think that either work fine along with the examples and I am NOT advocating that we need to switch back. Overall this is hugely helpful and the use of examples and "Menu router path" might even help me better understand this term which I do hear frequently.

bugfolder commented 2 years ago

As a follow-up, https://docs.backdropcms.org/documentation/deep-dive-building-with-blocks-and-layouts should probably be edited to use the same terminology.

(Actually, that page has some incorrect information on it: an "override layout" is not the same thing as a "dynamic layout". The former is a layout for an existing path; the latter contains a placeholder. A layout could be either (node or foo/%), both (node/%), or neither (foo).)

And perhaps we should try to standardize terminology for the various types of path throughout code and docs in a follow-up issue? (As we recently did with "URL Aliases".)

docwilmot commented 2 years ago

Oh dear, menu router paths is even less friendly to me. Perhaps I should ask for clarification as to who this help text is for; I believe it should at least target the people least familiar with such terminology as menu router paths.

My goal was to include all of the new concepts in a concise paragraph that painted a complete picture, before we started with the bullets.

I dont see that as necessary, the fieldset already lends context to the idea that this would be instructions on "how layouts work", and bullet points are easier to follow.

I dont feel the point re layouts being alphabetical lends anything much to this, which is why I removed it.

I would still combine "Each path may have one or more layouts available." and "Available layouts are listed beneath the path." into one sentence such as "Each Path listed below may have one or more available layouts assigned. For paths with multiple layouts:" because the second sentence is only relevant if the first is true.

Paths used by the layout system are menu router paths (example: node/%), not normal paths (example: node/2) or URL aliases (example: about)

My point here is that as a first introduction to layouts, this is confusing, not the least because it could be inaccurate. Yes you can actually create a layout with a path that looks like node/2 or officer/25 or about or cheese. Those examples may be misleading.

jenlampton commented 2 years ago

I dont feel the point re layouts being alphabetical lends anything much to this, which is why I removed it.

I added it specifically because some people were confused about how things were ordered on this page (and having it helped them) -- we can remove it if I was mistaken about that :) @stpaultim do you remember this?

I would still combine "Each path may have one or more layouts available." and "Available layouts are listed beneath the path." into one sentence

I had considered combining these into the same bullet also, but I don't like combining the sentences. Let's start with the single bullet and see if that's enough of an improvement.

Question: Do we need the indentation in the bullets? I'm wondering if that adds any value.

My point here is that as a first introduction to layouts, this is confusing

Yes, this is a very confusing part of layouts. (We have a bunch of other issues to help with the confusion too). Unfortunately, it's also critical for people to understand what should be used for a path.

The likelihood of someone using node/2 when they should have used node/% but misunderstood, is about 99.9%. That's more than enough reason to add a disambiguation to the first introduction to layouts.

Yes you can actually create a layout with a path that looks like node/2

You are correct -- but as I said above -- but that is a bug. The layout system was never intended to be used that way so it probably should not be encouraged :)

bugfolder commented 2 years ago

You are correct -- but as I said above -- but that is a bug. The layout system was never intended to be used that way so it probably should not be encouraged :)

And I have created https://github.com/backdrop/backdrop-issues/issues/5433 where we can address what we might do about that to discourage it in the future without breaking things from the past. (I do like the warning message that pops up on the layout page if you try it.)

docwilmot commented 2 years ago

Side note, its not a bug. Backdrop recognizes that youre about to do this, and throws a warning; if you persist, then you intended to do this, and its your site, and you have your reasons, thats not a bug.

But point is those examples are misleading! If I have 3 types of cheese and I make a layout for cheese/3 why are these instructions telling me I cannot create a layout that looks like cheese/3? This is what your example is suggesting: dont make layouts that look like xxxx/number. Even worse, dont make layouts that look like a word (about). I feel this will confuse.

jenlampton commented 2 years ago

Side note, its not a bug. Backdrop recognizes that youre about to do this, and throws a warning

We added the warning because we were shocked when we discovered it was even possible (since it was not supposed to be possible -- hence the bug).

This is what your example is suggesting: dont make layouts that look like xxxx/number.

That's correct. This is also what the warning is suggesting. This is still good advice. If you are an expert and want to accept the risk and do it anyway, thats fine. These instructions aren't for you :)

Back to formatting question, here's what it might look like without indentation: is this better?

Screen Shot 2022-01-03 at 3 20 43 PM
bugfolder commented 2 years ago

How about for the first sentence:

As a page is rendered, a Layout is selected as follows. The Path for the page...

Just to make it clear that the two sentences are not describing sequential things.

jenlampton commented 2 years ago

Here's an example with indented bullets - and the first paragraph back in bullets, closer to how it was earlier today:

Screen Shot 2022-01-03 at 3 38 05 PM
bugfolder commented 2 years ago

Nice, but nitpick: the first sentence isn't part of the list, so shouldn't be bulleted.

docwilmot commented 2 years ago

That's correct. This is also what the warning is suggesting. This is still good advice. If you are an expert and want to accept the risk and do it anyway, thats fine. These instructions aren't for you :)

Last try, just in case I am not being clear. The only time it is "wrong" to do this ( meaning paths that look like xxxx/number) is for entity paths, like node/1, user/2. To be clear: story/2022 is ok, cheese/3 is ok. Your example would make it seem (perhaps only to me) that a xxxx/number path is bad, because a new user doesnt know that the "node" part of a "normal path" like "node/2" is a special thing.

docwilmot commented 2 years ago

P.s. apart from this last point, I am happy with the last iteration.

jenlampton commented 2 years ago

Your example would make it seem (perhaps only to me) that a xxxx/number path is bad, because a new user doesnt know that the "node" part of a "normal path" like "node/2" is a special thing.

Lightbulb! I get what you were saying now. Thank you for saying it 3 times.

hm... In the example all references were for the about page. I wonder if there is a clearer way to indicate that?

jenlampton commented 2 years ago

Okay, here's another one with the first/last bullets removed, more indented bullet added (not sure if this is better), and another sentence attempting to clarify that the examples are all for a single page:

Note: Paths used by the layout system are menu router paths, not normal paths or URL aliases. For the About page, node/% is the menu router path, node/2 is the normal path, and about is the URL alias.

Screen Shot 2022-01-03 at 3 49 27 PM
bugfolder commented 2 years ago

👍. One more teeny thing: in the last sentence, should it be "For the default About page,...", just to make it clear that this isn't a requirement for any site that happens to have an About page (like a site migrated from D7)?

jenlampton commented 2 years ago

Okay, here's a PR: https://github.com/bugfolder/backdrop/pull/6

bugfolder commented 2 years ago

👍. Merged into current PR.

stpaultim commented 2 years ago

I added it specifically because some people were confused about how things were ordered on this page (and having it helped them) -- we can remove it if I was mistaken about that :) @stpaultim do you remember this?

For the record, I do believe that my own misunderstanding of the order in which layouts were evaluated was part of the reason we went down this path. I was falsely under the impression that all layouts were evaluated from top to bottom (and I'm sure I've passed along this mistaken impression in recorded presentations).

I think the new text is very helpful in rectifying this.