backdrop-issues icon indicating copy to clipboard operation
backdrop-issues copied to clipboard

[UX] Normal paths should not be accepted as layout paths on Configure Layout page

Open bugfolder opened this issue 3 years ago • 16 comments

Per https://github.com/backdrop/backdrop-issues/issues/4868#issuecomment-1003865992, Normal paths (e.g., node/1) should not be accepted for layout paths; they should be router paths (e.g., node/%). So validation should be added to the Configure Layout page to enforce this.

bugfolder avatar Jan 03 '22 15:01 bugfolder

What does that mean in practice? That I can't and shouldn't override a layout for a single node per path anymore?

What about existing usages of paths like that out there? Let them break? Update hook to switch the path and add a visibility condition?

And most of all ... why is this supposed to be a bug?

Another thought: what about views paths?

indigoxela avatar Jan 03 '22 16:01 indigoxela

Yeah, I agree with @indigoxela on all points.

docwilmot avatar Jan 03 '22 16:01 docwilmot

All good points. I just wanted to capture the assertion so it can be addressed in its own issue rather than sidetracking the original issue. Will leave it to @jenlampton, who made the comment, to address the points.

bugfolder avatar Jan 03 '22 17:01 bugfolder

I always considered that a feature: assigning a special layout per path to only one node with something like "node/11". And I'm pretty sure, I'm not the only one.

It never came to my mind that using this should be discouraged or ... "fixed". Why would we fix a feature?

indigoxela avatar Jan 04 '22 08:01 indigoxela

And I'm pretty sure, I'm not the only one.

Sure, me too.

findlabnet avatar Jan 04 '22 08:01 findlabnet

I always considered that a feature: assigning a special layout per path to only one node with something like "node/11".

Is it also a feature that if you do this, the node content vanishes from the rendered page? Seems like that behavior could be pretty confusing. (Although having the warning appear as you're creating the layout probably helps.) I'm just wondering if the people who attempt this are expecting/trying to achieve the resulting behavior:

  • totally replace the node page with this layout page
  • don't show any of the node content, even if the layout contains the "Main page content" block
  • totally ignore any access permissions on the node, but still use the URL of the node (either node/1 or its alias).

bugfolder avatar Jan 04 '22 14:01 bugfolder

@bugfolder so you want to "fix this bug", do I get this correctly?

indigoxela avatar Jan 04 '22 15:01 indigoxela

I think the current situation is not ideal, so yeah, I guess that means "fix." I don't (yet) have a clear idea of what should be done—hence the opening of the issue for discussion/proposal.

bugfolder avatar Jan 04 '22 16:01 bugfolder

So when / if this "bug gets fixed", how can I only use a specific layout, for example, for a unique landing page?

findlabnet avatar Jan 04 '22 16:01 findlabnet

@findlabnet, If you want to override the layout used for path "node/1" but still display the content of node 1, you would:

  • Create your layout with path node/%
  • Give it a visibility condition of type "Node: NID", and used "Node NID is" = 1.

If you want a unique landing page that doesn't display the content of node 1, then you just create the layout and give it whatever path you want to use for the unique landing page.

I can't envision a use case where you would want to use the URL node/1 but not have anything related to node 1 actually be displayed. In that situation, what would be the purpose of having node 1?

bugfolder avatar Jan 04 '22 17:01 bugfolder

I was thinking that when someone did create a layout this way, we could use a submit handler to switch it to the expected use: a layout at node/% with a visibility condition limiting it to the single node, either by ID or path.

That way nobody needs to learn a new behavior, but we deliver the feature we intended, that will work closer to how they expect.

See https://github.com/backdrop/backdrop-issues/issues/5539

jenlampton avatar Jan 04 '22 17:01 jenlampton

@indigoxela I think I'm in agreement with @bugfolder and @jenlampton ...this is not a "bug" per se, but it feels like we are giving people a gun to shoot themselves on the foot (and they would then think that "layouts do not work properly" in these cases).

I was thinking that when someone did create a layout this way, we could use a submit handler to switch it to the expected use: a layout at node/% with a visibility condition limiting it to the single node, either by ID or path.

I like that, but I'd add a message to explain what has happened behind the scenes. It'd be less "magic" that way.

klonos avatar Jan 04 '22 18:01 klonos

Maybe we should be calling it an "accident" instead of a bug :)

jenlampton avatar Jan 04 '22 18:01 jenlampton

In looking at this again, I see that there's already some validation in place for this sort of thing. If you enter node/1 for the path, this warning message appears:

warning

This seems to describe things pretty well; the only change I would suggest is to change the last phrase to "add a 'URL path' visibility condition for ...", to make it clear what kind of "condition" should be added.

With respect to this:

when someone did create a layout this way, we could use a submit handler to switch it to the expected use: a layout at node/% with a visibility condition limiting it to the single node, either by ID or path.

I respectfully disagree. Doing something different from what they've requested just doesn't feel right, and this warning message ideally heads them off from shooting themselves in the foot this particular way before they do it.

bugfolder avatar May 02 '22 02:05 bugfolder

In looking at this again, I see that there's already some validation in place for this sort of thing.

The message we added after we noticed that we allowed this "accidental" behavior. This issue is the one to "fix" the "accident". :)

Doing something different from what they've requested just doesn't feel right

Even if they just didn't know how to ask for it correctly?

I want soap, so I ask for "sopa" ... but then I get soup. I also get a message saying that this soup might not be what I want, but since I don't really understand anyway, and since It's looks kinda like liquid hand soap I might continue anyway, and end up frustrated.

Even if I do realize my layout mistake, and already understand that there is a difference between a page and a layout, I would also need to spend time creating it over again the right way - and without deleting this wrong example it still might not work. :/

Alternatively, If you knew that nobody wanted soup, you could give me soap, along with a nice message that the correct word for soap is "jabón". Then I'd both get what I wanted without additional work, and I'd learn how to do it correctly the next time.

The latter seems much more friendly to me :)

I can't envision a use case where you would want to use the URL node/1 but not have anything related to node 1 actually be displayed. In that situation, what would be the purpose of having node 1?

Exactly. There isn't a use-case for the "accidental feature" we created, so I don't think we should support it in core. Hence this issue, to make normal paths "not be accepted" via the Layouts UI anymore. (That's also why I called it a bug initially.)

But if we're going to remove this from core, there wont be a scenario where can deliver the soup at all. :)

jenlampton avatar Mar 19 '24 00:03 jenlampton

I realize we already have an issue to create the layout they intended automatically. So I guess this issue is just to prevent the one they didn't want from existing.

Adding this to the 2.x milestone because removing something needs to be well documented.

jenlampton avatar Mar 19 '24 00:03 jenlampton