Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
bugfolder opened this issue Jan 3, 2022 · 16 comments

Comments

@bugfolder
Copy link

Per #4868 (comment), 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.

@indigoxela
Copy link
Member

indigoxela commented Jan 3, 2022

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?

@docwilmot
Copy link
Contributor

Yeah, I agree with @indigoxela on all points.

@bugfolder
Copy link
Author

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.

@indigoxela
Copy link
Member

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?

@findlabnet
Copy link

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

Sure, me too.

@bugfolder
Copy link
Author

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).

@indigoxela
Copy link
Member

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

@bugfolder
Copy link
Author

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.

@findlabnet
Copy link

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

@bugfolder
Copy link
Author

bugfolder commented Jan 4, 2022

@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?

@jenlampton
Copy link
Member

jenlampton commented Jan 4, 2022 via email

@klonos
Copy link
Member

klonos commented Jan 4, 2022

@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.

@jenlampton
Copy link
Member

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

@bugfolder
Copy link
Author

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.

@jenlampton
Copy link
Member

jenlampton commented Mar 19, 2024

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
Copy link
Member

jenlampton commented Mar 19, 2024

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 jenlampton added this to To do in Layout system via automation Mar 19, 2024
@jenlampton jenlampton added this to the 2.x-future milestone Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Layout system
  
To do
Development

No branches or pull requests

6 participants