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

Navigation block: current menu item styles #42299

Open
Tracked by #33447
MaggieCabrera opened this issue Jul 10, 2022 · 53 comments
Open
Tracked by #33447

Navigation block: current menu item styles #42299

MaggieCabrera opened this issue Jul 10, 2022 · 53 comments
Labels
[Block] Navigation Affects the Navigation Block [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi Needs Design Feedback Needs general design feedback. [Type] Task Issues or PRs that have been broken down into an individual action to take

Comments

@MaggieCabrera
Copy link
Contributor

The user/themer should be able to style the current menu styles. It could be done something like this in theme.json:

Screenshot 2022-07-10 at 17 59 06

"core/navigation": {
     ":current": {
          "color": "red"
     }
}
@Marc-pi
Copy link

Marc-pi commented Jul 11, 2022

relative to #40778

@martinkrcho martinkrcho added [Type] Enhancement A suggestion for improvement. [Block] Navigation Affects the Navigation Block labels Jul 11, 2022
@getdave
Copy link
Contributor

getdave commented Jul 14, 2022

Whilst I recognise the need for this, I really don't think we should be using the pseudo selector syntax. AFAIK :current is not a valid CSS pseudo class. I feel we should reserve the :{selector} syntax for valid selectors only.

If we want to style current we should consider an alternative syntax.

@getdave
Copy link
Contributor

getdave commented Jul 14, 2022

Information I found says:

These pseudo-classes apply when viewing something which has timing, such as a WebVTT caption track.

I don't think that the currently selected Navigation element has "timing" in the sense the spec suggests. Have you seen a usage in the wild where :current is used in the context of Navigation?

@scruffian
Copy link
Contributor

I don't think its necessary for theme.json syntax to match the CSS spec. theme.json is an abstraction of CSS into a system that Global Styles understands, so it's totally valid for us to implement things in a way that works for block themes, even if it doesn't map perfectly into to the CSS spec.

@mtias

@getdave
Copy link
Contributor

getdave commented Jul 21, 2022

I really think that we're going to go down the wrong track here. We have an established expectation that the pseudo selector used in theme.json (e.g. :hover) maps to the equivalent in CSS (e.g. :hover).

What is being proposed here is that we circumvent this expectation for a single given use case (i.e. just because it's convenient for us to use :current to refer to a concept that is not a pseudo selector).

I appreciate we need to optimise for user experience over developer experience. However, I think in the longer term the proposed approach could open the door to more such confusing edge cases which folks will be required to memorise.

Let's take a moment to consider who we are really optimising for here.

We have two types of "user":

  1. A Site Owner
  2. A WordPress Developer

We can easily shield the "Site Owner" from any decisions we make in Theme JSON because we have the Global Styles UI acting as a layer between the code and the UI that they experience. Therefore the user we are actually optimising for here is the WordPress Developer.

IMHO asking developers to memorize random exceptions to what are otherwise clear cut conventions when authoring Theme JSON is poor UX.

Also consider that if we proceed with what's being proposed and then further down the line we want to add a :current pseudo selector for <video> elements then we'll be unable to do so. This is because we'll have "used up" the :current selector for another unrelated purpose and thus be unable to use the :current selector in the way it is intended.

My opinion is that we should come up with an alternative syntax to allow us to add these states within Theme JSON without impinging on the established convention and causing confusion.

It's just an idea but how about we utilise a @ to denote these kinds of states?

"core/navigation": {
     ":hover": {
          "color": "red"
     },
     "@current": {
          "color": "hotpink"
     }
}

To me this reads as "at the state when the item is the one which is currently active then apply these styles". What do you think?

A final note to say that if/when the "current" state is exposed to Site Owners in the Global Styles UI I fully expect it to be listed alongside other "states" such as :hover, :focus .etc.

@getdave
Copy link
Contributor

getdave commented Jul 21, 2022

@WordPress/block-themers Do you have any opinions on this proposal? I've outlined the arguments above and we need to come to a decision about whom we're intending to optimise for here.

IMHO whatever we do in Theme JSON should be optimised for developers not site owners. Site Owners experience styling via the Global Styles UI and thus we can easily "shield" them from whatever we implement in the raw theme.json.

@colorful-tones
Copy link
Member

IMHO whatever we do in Theme JSON should be optimised for developers not site owners. Site Owners experience styling via the Global Styles UI and thus we can easily "shield" them from whatever we implement in the raw theme.json.

Yes, I follow this logic.

I think using :current would be a mistake and has the potential to cause conflict further down the line.

I like @getdave proposal for using @current. This seems like a nice compromise and does clearly indicate state (in naming).

@madhusudhand
Copy link
Contributor

madhusudhand commented Jul 21, 2022

I agree that :current may not be appropriate and upvote the @ prefix.
Also I suggest @selected instead.
More than a state, I would consider it as a variant of a block, which can have states such as :hover inside of it.
It may not make sense to have :hover inside of @selected for a nav menu, but thinking in a broader syntax for the prefix @.

"@selected": {
   color: { ... },
   :hover: {
      color: { ... }
   }
}

Thinking out of the current context, for the blocks such as buttons that has variants such as fill and outline, does it make sense to adopt the similar syntax. Bringing it to discussion to for a more general context.

I mean use the prefix @ or other to indicate a special variant of a block. In case if we would like to style the outline buttons, the same prefix can represent it, instead of adding another prefix for such scenarios in future.

button: {
  color: { ... },
  @outlined: {  // or @filled
     color: { ... }
  },
}

@colorful-tones
Copy link
Member

More than a state, I would consider it as a variant of a block

I think we're digressing in mixing the discussions of variants and should focus the usage on state.

@mikachan
Copy link
Member

IMHO whatever we do in Theme JSON should be optimised for developers not site owners. Site Owners experience styling via the Global Styles UI and thus we can easily "shield" them from whatever we implement in the raw theme.json.

Completely agree with this, and would also like to +1 the @current syntax.

@draganescu
Copy link
Contributor

But what if we stick to @ for all of them? @hover, @current ?

@carolinan
Copy link
Contributor

carolinan commented Jul 26, 2022

This is for styling a menu item that already has a CSS class that indicates that it is the current page, correct?
Then can't it be:

"core/navigation": {
     ":hover": {
          "color": "red"
     },
     ".current-menu-item": {
          "color": "hotpink"
     }
}

@scruffian
Copy link
Contributor

"core/navigation": {
     ":hover": {
          "color": "red"
     },
     ".current-menu-item": {
          "color": "hotpink"
     }
}

My concern with this is that it might create an expectation that I can specify any class name here and expect it to work, which it wouldn't!

@draganescu
Copy link
Contributor

This styling should be for link elements inside the navigation block. So the syntax we arrive at should apply in a generic manner to all link elements IMO.

@carolinan
Copy link
Contributor

"core/navigation": {
     ":hover": {
          "color": "red"
     },
     ".current-menu-item": {
          "color": "hotpink"
     }
}

My concern with this is that it might create an expectation that I can specify any class name here and expect it to work, which it wouldn't!

Yes that is a valid concern: I don't find it unlikely that more (not all or custom) classes might be supported in the future.

@getdave
Copy link
Contributor

getdave commented Jul 26, 2022

But what if we stick to @ for all of them? @hover, @current ?

Then we're back to the original argument.

@getdave
Copy link
Contributor

getdave commented Jul 26, 2022

I've added this to the Core Editor chat agenda to see if we can get further input on this.

@draganescu
Copy link
Contributor

Then we're back to #42299 (comment).

My current opinion is that, while a great endeavor, we gain little from mapping CSS to theme.json in terms of pseudo classes. Having some props map to CSS via : and some not map via @ adds complexity. Few people know those by heart and we'll be introducing a great opportunity to write @hover and :current and wonder what's wrong.

I fully agree that using CSS style : for props that don't exist in certain contexts is wrong, hence my personal preference would be to apply uniformity and just use @ for all of them simply because we will possibly need other weirder states for block elements in the future and @ has it all covered.

Again, I do think that having both is ideal but at the same time having one syntax is simply a better experience.

@getdave
Copy link
Contributor

getdave commented Jul 26, 2022

Ah I see @draganescu. So you're suggesting we use @ instead of : in all cases (valid CSS pseudo selectors and made up WordPress ones) in order to make it clear that there is no mapping between CSS and Theme JSON in both cases.

That's an interesting alternative suggestion.

@scruffian
Copy link
Contributor

@ is not without its own meaning: https://www.w3.org/TR/css-syntax-3/#at-rules

@scruffian
Copy link
Contributor

Also consider that if we proceed with what's being proposed and then further down the line we want to add a :current pseudo selector for

I'm not sure this is a valid concern. The way we have been implementing :hover and :focus so far is on link elements. I don't think using :current to control aspects of a link element would prevent us from using :current to control other aspects of a block when used at a block level rather than a link level, would it?

@mrwweb
Copy link

mrwweb commented Jul 27, 2022

This definitely won't be the last time this issue comes up, and I agree that anything that creates a mismatch between CSS syntax and theme.json will add unnecessary mental overhead and—speaking for myself—decrease the likelihood of me going all-in on theme.json CSS. As @Marc-pi noted, .current-menu-ancestor is another requested class that could need styling soon and so whatever solution is used for this should ideally apply to that as well (or there's a plan for and reason why it will differ).

I'd much prefer @carolinan's suggestion. It maps to the CSS concept of .parent .child selectors, and I can imagine that extending it to support any core-related CSS class would make a lot of sense and could potentially solve a lot of similar issues like #40778. Both : and @—along with ~, +, ::, > and a few more I'm sure I forgot—have meanings in CSS, so they should be reserved for their intended uses, especially when other options are available!

I will also just call out that the item with this styling should have aria-current="page" on it which maps to the valid CSS selector [aria-current="page"]. There's a great argument that using selectors like that is a good way to ensure that the correct markup is in place, since it requires not just the class but the attribute that gives the class semantic meaning. That is a solution that's not generalizable to all classes, but it might spark an idea for someone else (and maybe there should be consideration for attribute selectors so that it's easier to style things like input[type="number"].

@getdave
Copy link
Contributor

getdave commented Jul 28, 2022

Ok so I’m hearing that @ might not be favoured. Is there an alternative syntax that isn’t part of the CSS lexicon that might work here to denote “states”?

Whilst I appreciate the logic and find the suggestion highly valuable, I also share @scruffian’s concern that allowing particular CSS classes to be introduced here might set the expectation that any CSS selector can be used.

This comes back to my point that we should avoid folks needing to understand too many “rules” about what is/isn’t allowed in certain contexts. In this case the developer needs to know that in specific cases you are allowed to use specific CSS selectors in specific places. I think that’s an undue cognitive overhead.

Maybe there’s a way where we can prepend or nest the selector in order to denote that it is a special case? For example:

“core/navigation”: {
     “:hover”: {
          “color”: “red”
     },
     “states”: {
        “.current-menu-item”: {
            “color”: “hotpink”
        }
     }
}

This clearly differentiates it from pseudo selectors whilst also helping to avoid the impression that it will accept any CSS selector?

We also need to consider that we might want to apply pseudo selector styles to the current item. For example, how can I style the current item on :hover? Or is this taking things too far? Should that be handled by custom CSS?

I am conscious this is far from perfect and I’m not making any decisions just outlining suggestions for consideration :man-bowing:

@scruffian
Copy link
Contributor

What about following the aria label and using :current-page?

@mrwweb
Copy link

mrwweb commented Jul 28, 2022

Whilst I appreciate the logic and find the suggestion highly valuable, I also share #42299 (comment) that allowing particular CSS classes to be introduced here might set the expectation that any CSS selector can be used.

It feels like this issue could really use some high-level discussion—I hope there has been some already though I haven't seen it (update July 29, 2022: here's a small discussion from last year)—about the overall planned scope of CSS support in theme.json (and therefore also global styles?). It's never seemed to me like it would be possible to support all of CSS, and therefore, it would make a ton of sense to discuss what the limits are.

One possible limit that might be useful would be limiting it only to elements and the CSS classes output by core (and possibly also registered block styles?). That would exclude things like parent context (e.g., .wp-block-column .wp-block-button), custom classes (.wp-block-button.wild-and-crazy-class), and more advanced relational CSS selectors (e.g. h2 ~ .wp-block-button:nth-child(odd):last-child) That also feels like it would align with the idea of "state" broadly enough as discussed in tickets like #38694 and #38998 and be a useful limiting principle.

If using that ideal, the states proposed by @getdave makes a lot of sense to me. I think I would just drop the . in the selector to reduce the similarity to CSS selector syntax:

"core/navigation": {
  ":hover": {
    "color": "red"
  },
  "states": {
    "current-menu-item": {
      "color": "hotpink"
    },
    "current-menu-ancestor": {
      "fontWeight": "bold"
    },
    "menu-item-has-children": {
      "border": {
        "style": "dotted",
        "color": "papayawhip",
        "width": "0 0 .25rem 0"
      }
    }
  }
}

The only downside is that :hover is also a "state" in a different sense. It would feel like putting hover/focus/active/checked/required etc in another object like "states" would be a good idea, if that ship hasn't sailed already.

@mtias
Copy link
Member

mtias commented Sep 10, 2022

I definitely think we should start by limiting to classes core wants to honor and commit to generating on its own. One of the issues for supporting any class is that we don't control the output of the classes, which would fail the contract we have on theme.json, which is things that are fundamentally supported across the board and for which you don't need to concern yourself on whether they are being outputted.

I don't quite like the "states" object given :hover also represents a state and is outside. It also seems one of those things you'd just forget over and over (was this a state or a root attribute descriptor?).

In that sense, this might be alright:

"core/navigation": {
     ":hover": {
          "color": "red"
     },
     ".current-menu-item": {
          "color": "hotpink"
     }
}

@philhoyt
Copy link

philhoyt commented Jun 14, 2023

I might be failing to understand the need for a restricted allowed class list. If we were able to target an element in the manner @mtias has mentioned without class limitation a theme author would be able to target additional block styles whether registered or otherwise that are currently inaccessible in theme.json

Imagine being able to register a style

if ( function_exists( 'register_block_style' ) ) {
    register_block_style(
        'core/quote',
        array(
            'name'         => 'blue-quote',
            'label'        =>  'Blue Quote',
        )
    );
}

And being able to target it

"core/quote": {
     ".is-style-blue-quote": {
          "color": "blue"
     }
}

I acknowledge the addition of variations in theme.json

"core/button": {
	"variations": {
		"outline": {
			"border": {
				"color": "#000",
				"radius": "0",
				"style": "solid",
				"width": "3px"
			}
		}
	}
}

But it is limited to some core blocks and seems so ad-hoc and limiting in theme development.
I guess an explanation of the following would go a long way in helping me understand the limitations

One of the issues for supporting any class is that we don't control the output of the classes, which would fail the contract we have on theme.json,... - @mtias

@mtias mtias added the [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi label Jun 29, 2023
@mtias mtias changed the title Navigation block: current menu as pseudo element Navigation block: current menu item styles Aug 2, 2023
@mtias mtias mentioned this issue Aug 2, 2023
66 tasks
@ellenbauer
Copy link

I think @jasmussen design proposal is a great, easy to understand solution for adding the functionality block settings. I would love to see this added to the Navigation block.

@jasmussen
Copy link
Contributor

jasmussen commented Sep 13, 2023

We discussed in Slack yesterday how tricky this issue is (link requires registration), but as a result of the conversation I took a quick stab again (Figma):

i4, global styles, medium prominence

This UI is present only in Global Styles, and what the mockup here shows is essentially to add style-variation-like buttons for special states for each block. In this case, for the Navigation block.

By showing those buttons in context of the preview, and towards the top of the inspector, all the panels below can be lower in the hierarchy, and therefore available to that particular selected state. And by being mutually exclusive toggle buttons, we can also explore a separate layer on top of this for the future, namely pseudo states (hover, active, visited, focus) that would be a separate layer on top of each. I.e. we might have a separate pseudo state dropdown, a la the below mockup, which would be available for both default and "active item" major states:

States

But as far as next steps, and if the mockup provided here gels with people, we might indeed start with the theme.json propsoal that Maggie suggested. The UI here should map nicely to it, and can always follow the theme.json implementation. What do you think? Can we ask a dev to start with the theme.json piece? If we're lucky, this can make 6.4 🤞

@getdave
Copy link
Contributor

getdave commented Sep 13, 2023

Just to clarify this is the API we are targeting

"core/navigation": {
     ":hover": {
          "color": "red"
     },
     ".current-menu-item": {
          "color": "hotpink"
     }
}

We will need to adjust the schema to accept this kind of value. In my mind it should be locked down to only allow certain "selectors" on certain blocks. I believe it will require modifying the sanitize method of the Theme JSON classes in PHP.

@getdave
Copy link
Contributor

getdave commented Sep 13, 2023

@draganescu and I will take a look into this tomorrow. Can't say whether it would make 6.4.

@fabiankaegy
Copy link
Member

Just noting here that I really hope we can open up the API for defining states in blocks also to custom blocks with custom states in the future :) the concept of for example the currently active item is a much needed element for tabs blocks for example.

@getdave
Copy link
Contributor

getdave commented Sep 13, 2023

I'm imagining this as creating the underlying code changes required to allow this same concept to be extended to other blocks in future. So there will be a whitelist of blocks -> selectors in much the same way as we have blocks to pseudo states.

@fabiankaegy
Copy link
Member

Yeah the reason why I mention it is because with pseudo states this currently isn't extendable or even usable by 3rd party blocks :)

@SaxonF
Copy link
Contributor

SaxonF commented Sep 14, 2023

FYI I've moved this to in progress as even though there is design feedback and work to be done I think focusing on theme.json first is a fine first release. We can maybe create a follow up issue that looks at how we should handle states in general (e.g. button hover etc)

@jasmussen
Copy link
Contributor

This issue is intentionally separate from styling pseudo states, both because one can happen without the other, but also because interface wise although they need to be considered together, ultimately you need to be able to style the hover color of the active menu item, so they are compounding. For just pseudo states, there's #38277.

@getdave
Copy link
Contributor

getdave commented Sep 14, 2023

My understanding is that the target Theme JSON we're aiming for would be something akin to:

"styles": {
        "blocks": {
            "core/navigation": {
                "elements": {
                    "link": {
                        ":hover": {
                            "typography": {
                                "textDecoration": "underline"
                            }
                        },
                        ":focus": {
                            "typography": {
                                "textDecoration": "underline dashed"
                            }
                        },
                        ":active": {
                            "typography": {
                                "textDecoration": "none"
                            }
                        },
                        ".current-menu-item": {
                            "typography": {
                                "textDecoration": "none"
                            },
                            ":hover": {
                                "typography": {
                                    "textDecoration": "underline"
                                }
                            },
                        }
                    }
                }
            }
        }
    }
}

My main question is how we nest pseudo states so that you can style the hover state of the currently selected item.

@getdave
Copy link
Contributor

getdave commented Sep 14, 2023

Alternative proposal as suggested by @MaggieCabrera and @scruffian

"styles": {
    "blocks": {
        "core/navigation": {
            ".current-item": {
                "typography": {
                    "textDecoration": "none"
                },
                "elements": {
                    "link": {
                        ":hover": {
                            "typography": {
                                "textDecoration": "underline"
                            }
                        },
                    }
                }                   
            },
            "elements": {
                "link": {
                    ":hover": {
                        "typography": {
                            "textDecoration": "underline"
                        }
                    },
                    ":focus": {
                        "typography": {
                            "textDecoration": "underline dashed"
                        }
                    },
                    ":active": {
                        "typography": {
                            "textDecoration": "none"
                        }
                    },
                    
                }
            }
        }
    }
}

@getdave
Copy link
Contributor

getdave commented Sep 14, 2023

Proposed API

@draganescu, @scruffian, @MaggieCabrera and I sat down away from Github and devised the following API and implementation:

  • modify Theme JSON API to allow .current-item (syntax TBC) as a top level item for any block in theme.json
  • allow blocks to determine what concept maps to "current" in their implementation. An example of this is how the Navigation (Item) Block does this:
    $is_active = ! empty( $attributes['id'] ) && get_queried_object_id() === (int) $attributes['id'] && ! empty( get_queried_object()->$kind );
    $wrapper_attributes = get_block_wrapper_attributes(
    array(
    'class' => $css_classes . ' wp-block-navigation-item' . ( $has_submenu ? ' has-child' : '' ) .
    ( $is_active ? ' current-menu-item' : '' ),
    'style' => $style_attribute,
    )
    );
  • instead of applying a hardcoded classname (e.g. current-menu-item) the block can query a central API (TBC) which returns the classname selector to be used for "current-item" in the given block. The block then applies that classname to it's "current" state.

Notes

We should investigate the Selectors API as this may allow us to define the mapping of "current-item" to the appropriate CSS classname on the per block basis via block.json.

@draganescu
Copy link
Contributor

A less implementation oriented explanation for the api:

Current item

We propose a new API that allows users to style the current item of a block exposing multiple items, when these items can become current in some way. Examples are:

  • the navigation block
  • the details block
  • a 3rd party tabs block
    The API to style the current item allows themers to style it in theme.json and users using the block editor UI using global styles and block supports UI.
    For themers there would be a new key named ‘currentItem’ available for every block.
//...
“core/navigation” {
  //...
  “currentItem”: {
    color: {
      //...
    }
  }
}

The CSS generated from the settings set up by current item JSON will target a special selector that the block defines via the selectors API. Therefore the block must support current item and also define the selector that desigrates it for the styling to apply.
For users, the block support and global styles UI will allow them to apply any style that the block supports to the current item.

Why current item

The reason why current item styling is something useful is because other items in lists (first, last, nth) are visible to users in the editor. If they want to style something on a visible position that is possible today via block styles UI.
However current is something that is invisible and cannot always be simulated while editing for various reasons. Adding a simple way to style these items are a powerful feature for customisation.
For example the navigation block can support current item both at the level of the sub menu and at the level of navigation link. So users and themers can style current submenu and current navigation link.

@mrwweb
Copy link

mrwweb commented Sep 15, 2023

@draganescu I have a few questions/feedback based on what you shared.

  1. The navigation block is definitely the easiest example to understand. It also has a one-to-one relationship with the aria-current attribute that should be going on the link in the .current-item list-item. Therefore, the other types of blocks where a "current item" makes the most sense to me are other situations where aria-current is appropriate: a breadcrumbs block, paging navigation, Page List block, and even maps and date pickers. There's a good argument for the CSS that implements these styles to use aria-current as a selector.
  2. I'm much less clear on what a "current" details block would be since there is no "one at a time" limit to details (at least for now, it's being debated for addition to the HTML spec). Does it represent the open attribute in HTML? Does it represent the :focus or :focus-within state (or both)? Maybe it event maps to :active? I honestly have no idea what styles / selectors would be generated for a details block's "current item".

Taking those two points together, it seems to me that:

  1. The .current-item style should probably just be called "current" and should only apply to things that use the aria-current attribute. That feels just as clear and also could make more sense in various 3rd-party contexts.
  2. Styling the Details blocks with the open attribute—if that's the intention, which is my best guess—is a totally separate concept and it would therefore make sense to provide an open style-able state for the Details block.
  3. Interestingly, the "open" state could also apply to the Navigation block as well. (I don't think that's come up on this issue, surprisingly.) for clickable top-level items that have been expanded to show their children. Therefore, I could see that the open style might map to both the open attribute for Details but also the aria-expanded attribute on buttons. Again, that would cleanly map to needs of other 3rd-party blocks as well since it has a direct connection to an existing HTML attribute.

Potential take-home point: Many interactive states have HTML attributes and/or CSS pseudo-selectors associated with them. (e.g. current, expanded, and even "grabbed") Since CSS styles and HTML are clearly defined already, it makes sense to base the Styles API on those whenever possible.

@richtabor
Copy link
Member

@jasmussen What do you think of leveraging the existing block style variation control in Global Styles? They're very close in theory already. We can improve both holistically then, if they're 1:1 — even something simple like right caret would help convey.

In future iterations, maybe a popover with the available controls (like duotone), instead of a whole different drawer/panel. Just thinking aloud. 🤔

CleanShot 2023-09-15 at 17 30 51

@fabiankaegy
Copy link
Member

The CSS generated from the settings set up by current item JSON will target a special selector that the block defines via the selectors API. Therefore the block must support current item and also define the selector that desigrates it for the styling to apply.
For users, the block support and global styles UI will allow them to apply any style that the block supports to the current item.

Hey @draganescu 👋

I may be misunderstanding this here so please tell me if this is not the case :)

When you say "the block support and global styles UI" I read that as the settings on an individual block and the settings in global styles. However, currently the Selectors API only gets used for styles configured in Global Styles. For example, when your block opts in supports.color.background and sets the color background selector to a custom class, that only applies for the background color defined in global styles. Not the one set on an individual instance of the block. (sadly =D)

Does this mean that the states (kind of like the already existing pseudo selectors) are only meant to work in the site editor as global settings?

If so I think that will be very limiting to this API because when we look at the navigation block, the most basic site usually has at least two navigations. One in the Header of the site and one in the Footer of the site. They almost never share the same styling for the currently active menu item because the background color they are often differs. Also, the navigation in the header often is considered the primary one and therefore often gets more visually interesting treatment applied.

@draganescu
Copy link
Contributor

draganescu commented Sep 18, 2023

Thanks for the questions they're very useful.

@mrwweb I think you're spot on with identifying the pseudoclasses and HTML and ARIA attributes that should be used to target what is considered "current" for the user. The API described here is not meant to supplant that.

The @current API is a way to define styling rules via theme json for theme developers and via the block supports and global styles UI for users. The aim is to have a unified way to address the notion of "current" - which then at implementation should follow closely exactly what you described.

To answer your points more directly the block author can and should make use of pseudoclasses, HTML and ARIA attributes when outputing the block's HTML not this @current API. The API only allows the block author to specify the selctor where the CSS rules will be output. The API does not generate the selectors too, because that would make it impossible too hard to cover all possibilities (e.g. in a navigation block current can be the submenu and also the link the user is on).

This is the current thinking at least allowing for maximum flexibility and also trying to keep the granularity in check - e.g. there is no way to make a good UX if we implement one tab per possible state). But if this API has gaps unseen let's work through them.

@fabiankaegy good call, I am hoping the selectors API will be used across all the features. The API allows the definition of what the selector is for and then any part of the system, global styles or block supports, should "get" that selector and work with it.

Simply put, the goal is not not limit @current to global settings but allow users and theme developers to style global blocks and elements and also local instances of each. If that requires "updating" other APIs there's no way around it.

@mrwweb
Copy link

mrwweb commented Sep 18, 2023

@draganescu

The @current API is a way to define styling rules via theme json for theme developers and via the block supports and global styles UI for users. The aim is to have a unified way to address the notion of "current" - which then at implementation should follow closely exactly what you described.

Thanks for the response. The point about each block specifying unique selectors is helpful.

What I am mostly trying to say is that I think the plan right now for the @current API as I understand feels like it is ambiguous and that it might make sense to introduce another state or two at the same time. You mentioned that the Details would also support a "current" style, and as a theme author, I don't understand what real-world state that would apply to.

If there were an additional @open API, then I could see how one or both would make sense for lots of blocks:

  • Navigation would support @current and @open
  • Details would support @open
  • Breadcrumbs block would support @current

My point about the CSS and HTML APIs is that aligning the theme.json API with existing HTML/CSS states whenever possible is a way to combat that ambiguity (regardless of the actual implementation).

@annezazu
Copy link
Contributor

Punting this from the 6.5 board due to lack of activity.

@getdave
Copy link
Contributor

getdave commented Jan 25, 2024

Thank you for updating here Anne.

Some additional context - during this release cycle, contributions were focused primarily on trying to make the Navigation Overlay (mobile view) customisable and also on improving the mobile breakpoint detection. This is why there was a lack of activity here. You can read more in the Tracking Issue updates.

@mtias mtias added [Type] Task Issues or PRs that have been broken down into an individual action to take and removed [Type] Enhancement A suggestion for improvement. labels Feb 7, 2024
@jasmussen
Copy link
Contributor

jasmussen commented May 7, 2024

Noting that there are mockups included in #38277 (comment) that try to address this issue as well. If developers subscribed could review, it would be appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi Needs Design Feedback Needs general design feedback. [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
Status: 📥 Todo
Development

No branches or pull requests