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

Try adding sticky position support to the template part block #47133

Closed

Conversation

andrewserong
Copy link
Contributor

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:

  • You can set the position directly on the template part block, without having to wrap the template part in a Group block
  • Improved discoverability of positioning for the template part block

Cons:

  • The sticky positioning is applied to the block attributes where the template part is inserted. This means that it does not apply to all instances of that template part across multiple templates. A user would need to go through each template and set it to the desired position type, which is likely not obvious. This is a similar issue that led to styling controls being removed from the template part block back in Remove color, spacing, and layout options for Template Part block #36571 — in short, if a user thinks they've made a change to the template part, but they've only made changes to the local instance of the template part within a particular template, it could be frustrating.
  • If we land on this approach and then later decide to remove it (as in Remove color, spacing, and layout options for Template Part block #36571) dealing with the backwards compatibility problem of sites that have already started using it could be a challenge.

⚠️ As pointed out in #47043 (comment) by @jameskoster, one of the issues with exposing controls at the template part level is that users might likely think that the setting they've made will apply across all instances of that template part across other templates, however it does not. One idea might be to allow setting things like positioning at the document level (so, globally within the context of a template part) — however doing that might not be simple to implement, and could also be tricky to make intuitive for the user.

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?

  • Opt the template part block in to using the sticky position block support

Testing Instructions

  • In the site editor, select a template part block at the root of the document.
  • You should see the Position panel in the inspector controls in the sidebar.
  • Selecting "sticky" should stick it to the top of the screen when scrolling the site. Note: if your theme has site-wide padding, you might need to set the Top value of the site padding to 0 for the header to sit flush at the top of the page.

Screenshots or screencast

Within the site editor On the site frontend
image image

@andrewserong andrewserong added [Type] Enhancement A suggestion for improvement. [Type] Experimental Experimental feature or API. [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Block] Template Part Affects the Template Parts Block labels Jan 13, 2023
@andrewserong andrewserong self-assigned this Jan 13, 2023
@jameskoster
Copy link
Contributor

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.

@jameskoster jameskoster added the Needs Design Needs design efforts. label Jan 13, 2023
@apeatling
Copy link
Contributor

apeatling commented Jan 13, 2023

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:

Screenshot 2023-01-13 at 10 28 47 AM

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

@jameskoster
Copy link
Contributor

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

I think it would need to be modified to only apply to template part blocks of the same kind (header, footer, general)

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.

@apeatling
Copy link
Contributor

apeatling commented Jan 16, 2023

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

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.

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.

This makes sense, so when that lands the apply globally option should only apply to the specific named template part (when also used elsewhere)?

@andrewserong
Copy link
Contributor Author

It would need to be more granular than that, imo, and apply only to the selected template part.

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:

  • Where would the root-level block attributes be stored? Since in this case, they wouldn't be stored in the block markup (e.g. <!-- wp:template-part {"slug":"header","tagName":"header"} /-->) as that exists in the parent document. If it were to be stored in post meta or something like that, then it raises another question:
  • How might we store that metadata for template parts that are shipped as part of a theme, would it live in theme.json as another key in the template part definition? E.g.
	"templateParts": [
		{
			"name": "header",
			"title": "Header",
			"area": "header",
			"attributes": {
				"style": {
					"position": {
						"type": "sticky"
					}
				}
			}			
		},

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.

@andrewserong
Copy link
Contributor Author

Update: I've had a play with conditionally displaying a notice for Sticky if the block is non-root over in: #47207

@richtabor
Copy link
Member

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

@richtabor
Copy link
Member

A user would need to go through each template and set it to the desired position type, which is likely not obvious.

Agreed.

@jameskoster
Copy link
Contributor

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

  • It surfaces sticky regions as an option in the Inserter which can be useful.
  • It solves the "Update List View to flag in the UI when a block has a position value set" problem listed in Fixed / Sticky position follow up tasks #47043.
  • It avoids all of this complicated shenanigans around template parts.

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:

Screenshot 2023-01-17 at 10 58 45

But that is just an initial thought. I'd say this needs some more design consideration overall.

@apeatling
Copy link
Contributor

[...] And if you do edit the background - you've edited the background color of the header across your site.

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.

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:

The menu option does seem like a straightforward solution and worth a PR to try it out. 👍

@andrewserong
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Template Part Affects the Template Parts Block [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi Needs Design Needs design efforts. [Type] Enhancement A suggestion for improvement. [Type] Experimental Experimental feature or API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants