-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Try adding sticky position support to the template part block #47133
Try adding sticky position support to the template part block #47133
Conversation
Thanks for opening the PR. This is an important flow, and attaching the control to the template part block is quite possibly the most intuitive solution. But it would be a real shame to repeat mistakes of the past, especially considering the backwards compatibility shenanigans you mentioned in the cons section. Imo we shouldn't merge this until we've addressed the issues around clarity. The user should be able to determine exactly where the Position setting will be applied, and at the moment that is simply not the case. Perhaps in the short term there's something specific we can do just for this control – maybe it just needs an accompanying Notice that explains what's going on. But if we can come up with a holistic solution, that opens the door to adding other controls to the template part block as well. In any case this needs design. |
Thanks for writing this up and putting something together for us to try. I agree with you both that we should be very cautious here because of back compat. Trying this PR, one thing came to mind where we might already have a solution? We have an option to push all styles globally, meaning this could be used to apply all local template part design settings to all template part blocks: I think it would need to be modified to only apply to template part blocks of the same kind (header, footer, general), but it could work? If we believe this could be a suitable option, then I suspect it will still need to be surfaced at the time of selection, perhaps through some help text under the position selection box to let the user know about the option? "To apply position settings to all header template parts across different templates, select 'Apply globally' from the Advanced menu below." |
A UI like that might work, though it might need to be more prominent in this context (the one in the screenshot above lives in the Advanced panel).
It would need to be more granular than that, imo, and apply only to the selected template part. In that sense, #42154 would really help add some more clarity to this flow. |
What do you think of my suggestion to add some help text under the position control to point it out? This probably wouldn't scale, but might be a first step.
This makes sense, so when that lands the apply globally option should only apply to the specific named template part (when also used elsewhere)? |
I agree, if you're making changes that apply across usage of the template part, then it would make sense for it to be attached to that particular template part, rather than across all template parts of that type. There are a couple of further issues that would need to be figured out if we were to enable storing data at the individual template part level:
I imagine implementing that kind of thing is probably quite complex, and potentially part of the broader work on patterns as sectioning elements? #39281 Personally, I think something like that would be very cool, but likely not something we can really pursue in time for 6.2 🤔 I had another idea, if we can't come up with a viable path forward for this template part PR, which would be to try out hiding the Position UI on the Group block (or the Sticky option in general) if the block has parents. Or, display a warning if the block is set to Sticky but has parents. I might have a play with either of those ideas to see if there's anything viable there. |
Update: I've had a play with conditionally displaying a notice for Sticky if the block is non-root over in: #47207 |
Another issue with applying the position to the template part (in the context of headers) is that you'd likely need to edit the background color of the group block within the template part anyhow. Otherwise you end up with a transparent background on the sticky header. And if you do edit the background - you've edited the background color of the header across your site. CleanShot.2023-01-17.at.16.02.56.mp4 |
Agreed. |
@andrewserong those technical points further indicate to me that this property shouldn't be attached to the template part. It just doesn't make sense – a template part (or any block for that matter) in isolation has no context for position. I've said it before, but it's the responsibility of the document to position its blocks. Exposing the control while template editing only introduces the confusion around whether the setting is applied globally or not. All of this puts us in a bit of a catch22. I still think a Group variation with Position: Sticky makes sense, and warrants further investigation.
There are plenty of options to explore that make it easy to stick a header. It could be something as simple as adding a "Make sticky" option to the block menu: But that is just an initial thought. I'd say this needs some more design consideration overall. |
This is a good point, and highlights the awkwardness between local and global changes depending on where and in which context the changes are happening. You could imagine a user changing the position setting on the template part and it doesn't apply across the site, then changing the background color on the group and it does apply across the site. It would be incredibly confusing.
The menu option does seem like a straightforward solution and worth a PR to try it out. 👍 |
I'll close this one out now that we're only allowing sticky position for root level blocks (#47334). Separately, I'd still like to have a go at adding a "Make sticky" button in the block menu — will see how far I can get with that idea next week. |
What?
Part of #47043
🚧 🚧 🚧 🚧 🚧 This PR explores an idea, but it may not be suitable to land 🚧 🚧 🚧 🚧 🚧
This PR explores opting the Template Part block into sticky position support. This is a potentially contentious issue! Some of the trade-offs are described in Why. The intention of this PR is to have a PR we can play with to then discuss the pros and cons, and hopefully learn a little more about what the desired end goal might be... I am very happy to close out this PR if the consensus is to not add more styling controls to the template part block at this time 🙂
Why?
The sticky position block support sets a block to sticky in relation to its immediate parent. In order to create a sticky header, without enabling sticky positioning on the template part block itself, this means that we need to wrap a template header part in a sticky Group block in order for it to work.
In this PR, we experiment with enabling position support directly on the template part block. There are some pros and cons with this approach:
Pros:
Cons:
In looking at this PR, part of the goal is to determine what we think might be the best course of action. My hope is that we can come up with a solid direction for next steps whether or not this particular PR lands!
How?
Testing Instructions
0
for the header to sit flush at the top of the page.Screenshots or screencast