-
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
[WIP] Layout: Explore adding in sticky position support within the layout support #44723
Conversation
Size Change: +15 kB (+1%) Total Size: 1.3 MB
ℹ️ View Unchanged
|
I've experimented in b4d0801 with adding position output to the layout support, so that the rules are output as part of those rules (and so we can add additional rules as needed). It seems to be working okay in the editor for now (with a hard-coded magic value for Will continue hacking on this tomorrow and have a go at adding the server-side output, too. |
b4d0801
to
27b2a43
Compare
In 27b2a43, I've added in server-rendering support, which also factors in the admin bar for the top position when using a fixed or sticky position type: A couple of further thoughts from that change:
|
I'm wrapping up for the week, and busy with other things next week, so will pick this up again after that. Just jotting down some additional notes, after brainstorming a little with @tellthemachines and @ramonjd. Tasks to try next:
|
Testing this PR I noticed that changing the "Inner blocks use content width" setting resets both the Area and Position settings. |
Thanks! I'll fix that up. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this most interesting experiment!
My first attempt at testing didn't work, and it took me a while to work out that the sticky position only works if the container of the sticky block is bigger than the sticky block itself. This can become a major usability obstacle because the block nesting structure has to be just so for it to work as expected. I'm not sure what the best way to tackle this might be, but some thoughts:
- add some guiding text when "sticky" is selected as an option, to the effect that the block will only stick to its immediate container;
- could we only enable this support for blocks that aren't the only child of their container? Or even only for top-level blocks?
- build some custom sticky functionality with JS instead of the native CSS? 🙈 Clutching at straws here 😅
lib/block-supports/layout.php
Outdated
'declarations' => array( | ||
'position' => $position_type, | ||
$position_side => $offset_value, | ||
'z-index' => '250', // TODO: This hard-coded value should live somewhere else. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we get away with a lower value? In testing it seems to work fine with 2
. In the editor the current value is interfering with the toolbars and block appenders so in any case will need a bit of tweaking!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we can definitely play with the value here. I'll have a play with a lower value 👍
isBlock | ||
> | ||
{ POSITION_OPTIONS.map( ( option ) => ( | ||
<ToggleGroupControlOption |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The interaction here feels a bit awkward, I wonder if it could just be a toggle to "stick block to top or bottom" which when toggled on would show the area controls?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, good point! Yes, happy to play with the design. In this PR I was mostly copying what's in the screenshots in #30121, but it's pretty bulky having both an Area and a Position section. It'd be good if we can come up with a way to group them together. I like your idea of a toggle... I wonder how it'd work if and when we have fixed
as an option in addition to sticky
?
Thanks for taking this for a spin @tellthemachines!
Good thoughts, there! I think it'd be good to see how far we can get with native CSS output rather than adding in JS, and try out some of the suggested ideas about when we do or do not show the controls. While the current use case is thinking of a global header or footer, another potential one is for headings in a glossary that stick to the top as you scroll down a really long page. Having a block that's intentionally within the post content area of a page template could be useful in that sense, so that it's not globally sticky. However allowing that kind of thing in a user friendly way is probably not simple! Another thought is that To expand on some of your suggestions, some other potential considerations for the controls:
In terms of usability, there's also the idea that if we refactor the Layout panel to use the ToolsPanel, then we might have these controls be hidden by default. In that case, folks would (hopefully) only be reaching for them when they need them, which might possibly help mitigate the usability issues a bit, though that's probably not safe to rely on 🤔 Thanks again for the thoughts and testing everybody! |
b5f84d0
to
0b7cbb2
Compare
I've pushed a couple of code quality changes in 0b7cbb2. This flattens the data a little, and moves it over to the <!-- wp:group {"style":{"layout":{"position":"sticky","top":"0px"}},"backgroundColor":"vivid-red","textColor":"background","layout":{"type":"constrained"}} -->
<div class="wp-block-group has-background-color has-vivid-red-background-color has-text-color has-background"><!-- wp:paragraph -->
<p>A paragraph</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group -->
This structure also allows us to add other values in the future, if we wish to set them at the block level. E.g. Still to do from the earlier to-do list (in addition to exploring the more recent feedback):
|
631f742
to
81ea70e
Compare
This is testing very well manually, thank you @andrewserong !! I created a new template! 2022-10-28.15.48.01.mp4Maybe you know about this, but this is what the layout controls look like in Global styles: |
1b8e1b5
to
a106a00
Compare
Update: I'll be AFK off and on over the next couple of weeks, so just jotting down some notes and thoughts for next things to work on with this PR, for when I come back to it:
|
Nice work on this so far. Sticky positioning is working well for root level group blocks on the frontend, but I did notice that the effect isn't present in the editor.
I'm not convinced we should do this, and would not consider it a blocker to this PR. My general feeling is that position is an abstract concept that can be applied to any block within the context of a document. That is to say: it's the responsibility of a template to define whether its header is sticky or not. One thing that seems important is for the values in the UI to match the CSS behaviour. IE a "Sticky" block should effectively just have |
Just want to check in as I'm doing the weekly update for phase 2 tasks -- do you need any extra help or back up considering the on and off AFK? Would love to make momentum continues. |
Thanks for checking in @annezazu — Joen and James have added useful feedback for when I’m back, so I plan to dig into those ideas. There are a few things to figure out with this proposed feature, as I imagine we’re going to encounter quite a few edge cases! The main thing that I think would help with momentum would be for folks to test out this PR, have a go at setting some Group blocks to “sticky” and report back usability thoughts and feedback, that’ll help give a fuller picture of the edge cases that’ll need to be addressed. A good use case for folks to test is “I want a site with a sticky header, can I achieve this by setting a Group block in the header area in the site editor to use sticky + top position”. If the answer is “no”, then let’s figure out the missing things required to getting it working acceptably. In terms of help with the code side of things, if anyone is free to jump in and wants to work on the code — feel free to fork this PR or commit directly to it, though I understand it can be tricky to jump into a WIP, so I’ve got some other suggestions below. I’ll comment here again when I’m back, so happy to work with anyone who might’ve picked anything up. However, the most useful things to look at I think would be:
Hope that helps! Let me know if you need any more info here. |
Testing Before:After:Adding sticky to Header. Sticky did not work in the backend or frontend. This is what I looked for. |
Could we amend that to simply: "I want to make a Group block stick to the top of the viewport as I scroll past it"? Throwing template parts (particularly footers) into the mix at this stage is making things overly complex. Adding the Joen updated #30121 with the latest mockups. |
Great point @jameskoster. Yes, I think for this PR let's limit the scope to the Group block, whatever we can do to implement this incrementally sounds good 👍.
Awesome, thanks for that. Just left a follow-up comment over there to make sure I understand the goal for the initial implementation 🙂
Thanks for testing @paaljoachim! I think in that example it's to do with sticky being applied at a non-root level so the area that it affects isn't site wide. I think for the moment this means that (at least with this PR) we're better off pursuing the suggestion James made to just focus on the Group block at the moment, rather than template parts. Then, once this PR lands we can look at the flow for switching on position for template parts (or a group block that is a parent of a template part) in a follow up. |
Update: I've updated this PR and simplified the UI controls to match the latest designs in #30121 — toggling between Default and Sticky now also sets It does still take up a fair bit of room for a control that will (most likely) be used only in a couple of places, so I've progressed forward with the idea of converting the LayoutPanel over to using the ToolsPanel in a separate PR over in: #45833 — that seems to have been going pretty well, the only major issue so far is that there isn't quite enough room between the Justification and Orientation controls for the gutter. I've added a screenshot in this comment: #45833 (comment) As for the remaining issues in this PR, next up I'll take a look at:
|
|
||
if ( position === 'sticky' || position === 'fixed' ) { | ||
// TODO: Work out where to put the magic z-index value. | ||
output += `z-index: 250`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could the z-index be defined in theme.json layout.definitions
, e.g., is-layout-sticky
similar to is-layout-flow
?
🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just jotting down a note here that it might make sense to have some preset-like values for z-index
stored in settings.layout.zIndexSticky
or if we want support for multiple z-indexes, then perhaps an array of z-indexes for different purposes. That way we could have a core default z-index value for sticky / fixed position, but themes could then override it with their own values if need be.
Thanks for updating this, it seems to be working well. We probably need to do something with the helper text – at the moment it's only describing the sticky option. For now it may even make sense to use a switch here: I think this may be more intuitive. I acknowledge that it prohibits fixed positions, but I don't think that's a big problem for now, as that use case needs further design consideration. What do you think @jasmussen ? |
Thanks for the PR, thanks for the ping, taking for a spin and seeing the toggle work really well: This is super nice.
But perhaps we don't need the "Position" subheading. Just like how there isn't a "content width" subheading for this: — just the toggle, and maybe it could say "Stick to the top"? |
So, thinking more about this, and from good advice from a friend, it behooves us to consider an interface that can at least scale to additional properties in the future, and perhaps it would be frustrating to see the toggle replaced with a segmented control at a later time. So I tried one more option:
As far as near term, we could still go with just "Sticky" as a starting point, and have "Default" and "Sticky" options inside the segmented control. Or we could simplify the positional control with something like this: But it would still be useful to consider a design that can naturally evolve to the additional controls. Let me know your thoughts! @jameskoster I'd love your thoughts on this. It breaks a few rules 😅 Figma |
Thanks for the extra discussion! Happy to implement whichever of the designs folks think will work for the first iteration of the feature. I think if we use a single on/off toggle, then I agree that removing the Position label sounds good. If we retain the I like the idea of the X and Y position offsets, too — that'll allow some pretty cool advanced behaviour further down the track. To begin with, though, I reckon if we can keep it to "switch to sticky sets it to |
@jasmussen It all depends on how far we want to take this, which isn't 100% clear to me. Do we want a control(s) that is able to entertain the full gamut of css I'm happy to work on whatever, but it's quite a big task and will be tricky to get right. That's kind of why I suggested the simple toggle to begin with, but maybe it's not useful enough on its own? |
I think the answer is that it isn't entirely clear how far we'd want to go, for example my current instinct is that we should avoid "relative" as an option, and ideally provide an interface that makes you not need to worry about relative at all. I was also reminded that in one design we have items in all 4 corners, like this: How's that accomplished? Is it a two full-width flex-between navigation bars, top and bottom fixed respectively? Or is it 4, each of which is fixed to a corner? And are those positioned using a simple cover-block like matrix, or are those easier to accomplish with x/y coordinates? Or even top/right/bottom/left inputs? We don't have to accomplish all of this, I still think it's a great idea to start with as little as possible, e.g. sticky as it doesn't require any positioning. But what I realized is that it would probably be a better interface if what we ship today can naturally evolve into what we might ship next year. I.e. if we know we'll need a segmented control in the future, perhaps we shouldn't start with a toggle — even if initially that seemed like a simpler design. What do you think? |
My point is simply that I'm not convinced we do know that. But if your conviction is strong then let's stick with it because I haven't explored this issue with a microscope just yet 😁 Otherwise we have two options:
I have no strong feelings either way. |
That's the thing, I don't know for sure! But it's the best of the options I came up with, mainly because it's hard to find icons to denote the nuance between "fixed" vs. "sticky". A dropdown as also explored previously is another option: Mainly my point is, I would think we have reasonable confidence that we do want to add fixed (plus some positioning controls) in the future, and in that case, the toggle wouldn't scale. |
Okay. The only other problem I see is that the segmented control needs to include at least 2 values, so we'll need to either:
Did you have a preference? |
Right, that's the challenge of the moment, where "Default" really doesn't make too much sense if the control is a non-default control. But I think we can probably include it regardless, even without Fixed, — just like how you can add the Dropcap control and toggle it off. Then when we get a chance to add Fixed in the future, we can choose whether we want 3 options, or can do without the Default at that point. What do you think? |
Great questions and discussion, folks! Just my 2 cents, but since it's very easy for controls to become quite complex as we consider more advanced use cases, I think I'd lean toward a simple control to begin with (focusing on the main use case of stick-to-top), accepting that we'll likely wind up exploring a very different control at some point next year when we attempt the more complex use cases.
Good question — I suppose we can have the help text be descriptive of the current state, and have it say something different depending on the option currently selected, a bit like we do for the help text next to the "Inner blocks use content width" control? |
I opened a discussion here to further discuss the design. |
Thanks for all the feedback on this PR, folks! Based on the latest discussions, and since the technical implementation needed to change a bit to decouple from the Layout block support, I've opened up a separate PR for continued exploration over in: #46142 — that PR is now the current one for future work on this feature (with discussion happening over in #46032), so I'll close out this PR now. |
What?
🚧 🚧 🚧 🚧 🚧 WIP: This is nowhere near ready for testing yet 🚧 🚧 🚧 🚧 🚧
This PR is an early exploration into storing
position
and related data as part of the Layout object, in an effort to come up with a solution for #30121. This expands on ideas earlier explored in #38039.Related to: #30121
Why?
It'll be useful for many kinds of sites to be able to set one or two group blocks or template areas to be
sticky
positioned in order to anchor a header or footer to the top or the bottom of the page.How?
TBC
Note: much of this is hacked together for now — all the changes in 6.1 files should be moved elsewhere prior to stabilising the PR.
To-do
attributes.layout.position
value to style output in both the editor and on the site frontend (e.g. in the layout output inlayout.php
)layout.position
to be an object, as there are likely related values that need to be stored. For example, we need the position type (e.g.sticky
) as well as the particular edge it should be close totop
,bottom
, etc. Also, we might want an offset value if elements are to be set slightly off from being adjacent to the edge of the viewport.z-index
value to ensure that the sticky positioned element sits where it should. Edit: at the moment this is hard-coded.z-index
and ensuring that the element can be reached appropriately via List View, clicking on the element, etc, and all inspector / toolbar controls are still behaving as expectedscroll-padding-top
when setting a header in the site editor (e.g. this rule)position
object in block attributes. Should it live in thelayout
object, or in thestyle
object? There are trade-offs either way.Testing Instructions
For testing purposes, for now, let's focus on looking just at the Group block
Screenshots or screencast
WIPs: