Archeo: Fix nav block blockGaps values #5671
Conversation
There was a problem hiding this comment.
This is strange. So at one point a variable value worked for blockGap?
I've been thinking about variables in patterns recently and more generally. I think whenever we provide a custom value in a pattern or template, we should provide a fallback value as well. I think that will help down the line when we want to add some of these patterns to the directory, and when custom templates and parts are preserved on theme switch.
That said, adding a fallback value (e.g. var(--wp--custom--spacing--small, 2.5rem)) doesn't work here either. Maybe worth reporting a bug upstream to investigate why?
| @@ -9,7 +9,7 @@ | |||
| 'content' => '<!-- wp:group {"layout":{"inherit":"true"}} --> | |||
| <div class="wp-block-group"><!-- wp:group {"align":"wide","layout":{"type":"flex","justifyContent":"space-between"},"style":{"spacing":{"padding":{"bottom":"var(--wp--custom--spacing--medium)","top":"var(--wp--custom--spacing--medium)"}}}} --> | |||
| <div class="wp-block-group alignwide" style="padding-top: var(--wp--custom--spacing--medium); padding-bottom: var(--wp--custom--spacing--medium);"> | |||
| <!-- wp:navigation {"layout":{"type":"flex","setCascadingProperties":true,"justifyContent":"left"},"overlayMenu":"never","className":"site-footer","style":{"typography":{"fontStyle":"normal"},"spacing":{"blockGap":"var(--wp--custom--spacing--small)"}},"fontSize":"small"} /--> | |||
| <!-- wp:navigation {"layout":{"type":"flex","setCascadingProperties":true,"justifyContent":"left"},"overlayMenu":"never","className":"site-footer","style":{"typography":{"fontStyle":"normal"},"spacing":{"blockGap":"2.5rem"}},"fontSize":"small"} /--> | |||
There was a problem hiding this comment.
I also tested with the clamp(20px, 4vw, 40px) value and it doesn't work either.
|
Thanks for taking a look, I was worried it was just my environment!
Yes, I'm sure this was working last week when we were working on this PR #5640, but now I'm wondering if I'd messed up my GB environment whilst working on it.. (and it was incorrectly working for me) I really like the idea of adding a fallback value when we use variables, that makes a lot of sense. I did take a look at GB issues/changes, it looks like support for blockGap had been removed for the Navigation block and then more recently added back in, but I think variable support was removed completely. I'm not sure I'm understanding the changes correctly though, so spending some time with them now. Here are some relevant links (I think): |
|
I see, thanks for the context! I wonder if it has to do with the sanitization for the blockGap attribute, since both clamp and var do not work. |
|
I think it doesn't work at the moment and you have to use CSS :( I played with it a bit here: WordPress/wordpress-develop#2307 |
Yeah I think it could be something like this.
Ah OK, thanks for confirming. Hmm, should we pick a value that works in the markup (like the |
But why does it work when we supply a non-dynamic value like rem and px for the blockGap attribute?
I think the baked value approach is preferable. So this PR LGTM as long as @kjellr is okay with the |
|
Oh I wasn't aware of that. Would you mind opening an issue for it? |
Done here: WordPress/gutenberg#39583 |
|
This looks good. It appears that the spacing I implemented in 63f02df has been lost at some point, so I'm going to open a quick followup PR to fix that. |
Changes proposed in this Pull Request:
This PR changes the blockGap values in the header and footer templates from
var(--wp--custom--spacing--small)to2.5rem.We've only recently changed to the variable in this PR #5640, however, I can't see this working locally anymore and I don't think this is working on dotcom. The
remvalue seems to work consistently both locally and on dotcom. Here's what it looks like:Footer nav at 400px:
Footer nav at 1200px:
Header nav at 1200px:
Related issues:
Probably related to #5632