There are two components that have already been built in Codex that relate to MenuItem:

1. ListTile, which represents the content and styling of the component
2. Option, which represents the behavior of the component within the context of a menu

We may be able to combine these two existing components into a single component, MenuItem, that fulfills all of these needs and allows library users to add custom content via the default slot (as is currently possible with the Option component)

Now that we are in the process of re-working our existing Menu logic into a shared Menu component (which will live inside Select, Combobox, TypeaheadSearch, etc), I would be in favor of this. Having a single "MenuItem" component seems like a sensible goal.

@Sarai-WMDE question for you: when should the "body" in the design be bold? Should this be based upon some rules (e.g. if descriptions are shown) or a prop (e.g. the body text is normal unless a specific prop is passed in, e.g. boldLabels or something, which will set the font weight to bold)

Change 769101 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[design/codex@main] MenuItem: Change Option to MenuItem

https://gerrit.wikimedia.org/r/769101

Change 769111 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[design/codex@main] MenuItem: Merge in ListTile and reflect updated designs

https://gerrit.wikimedia.org/r/769111

Change 769114 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[design/codex@main] docs: Improve demos of components that use menus

https://gerrit.wikimedia.org/r/769114

@Sarai-WMDE I have some questions for you when you're back in the office!

Menu item label

When should this be bold or not bold? I had two ideas:

• Add a boldLabel prop (or some better name) so the developer can control this
• Make it automatic within the MenuItem component: default to not bold, but if there is a description, make the label bold. Note that this wouldn't apply when we're highlighting the search query, e.g. for TypeaheadSearch.

What do you think?

Text overflow behavior

Again, should this be controlled via a prop? Or props, since we might want to implement different behavior for the label vs. the description? I would suggest we default to showing the entire label but hiding description overflow via ellipses. From there, we can either add a prop to change the behavior for the description and possibly also the label, or we could just assume that library users can use CSS to change the behavior themselves.

TypeaheadSearch keyboard nav

Now that ListTile has been made more general for wider usage by incorporating it into MenuItem, there's an effect on the TypeaheadSearch component that I wanted to run by you. When you navigate through menu items via keyboard, you'll notice that the highlighted item's text is blue: this is because the item is technically both selected and highlighted. Is this the desired behavior here, or should we write some special styles for TypeaheadSearch to keep the text black in this case? You can see it in action here: https://769114--wikimedia-codex.netlify.app/components/typeahead-search.html

You can test out the patches related to this task here: https://769114--wikimedia-codex.netlify.app

Does the spacing between MenuItem Icons and labels/descriptions look a bit tight to anyone else?

This is currently set to a value of 8px, which matches the Figma spec. But I wonder if we should consider expanding it slightly.

Change 769101 merged by jenkins-bot:

[design/codex@main] MenuItem: Change Option to MenuItem

https://gerrit.wikimedia.org/r/769101

In T300625#7761931, @egardner wrote:

Does the spacing between MenuItem Icons and labels/descriptions look a bit tight to anyone else?

This is currently set to a value of 8px, which matches the Figma spec. But I wonder if we should consider expanding it slightly.

@egardner what we currently have in the MenuItem Figma spec sheets is 8 px between icon and text in MenuItems. If we decide to update it we should open a new task to apply these changes in all the MenuItems variants.

Hey, @AnneT 👋🏻 Thanks for the questions! Let me answer inline:

Menu item label

When should this be bold or not bold? I had two ideas:

• Add a boldLabel prop (or some better name) so the developer can control this
• Make it automatic within the MenuItem component: default to not bold, but if there is a description, make the label bold. Note that this wouldn't apply when we're highlighting the search query, e.g. for TypeaheadSearch.

I would go for the first option and add a prop to allow making the default regular labels bold.

Text overflow behavior

Again, should this be controlled via a prop? Or props, since we might want to implement different behavior for the label vs. the description? I would suggest we default to showing the entire label but hiding description overflow via ellipses. From there, we can either add a prop to change the behavior for the description and possibly also the label, or we could just assume that library users can use CSS to change the behavior themselves.

My suggestion here would be to 1. apply the same behavior to both text elements: showing their entire content by default + 2. add a prop to allow hiding the description's overflow via ellipsis. It's in general not recommended to hide options and actions from users, so deciding to hide the label content should not be encouraged by the system (overrides are, of course, out of our jurisdiction).

TypeaheadSearch keyboard nav

Now that ListTile has been made more general for wider usage by incorporating it into MenuItem, there's an effect on the TypeaheadSearch component that I wanted to run by you. When you navigate through menu items via keyboard, you'll notice that the highlighted item's text is blue: this is because the item is technically both selected and highlighted. Is this the desired behavior here, or should we write some special styles for TypeaheadSearch to keep the text black in this case? You can see it in action here: https://769114--wikimedia-codex.netlify.app/components/typeahead-search.html

Thanks for the attention to detail, and for sharing the Netlify link! 😄
The desired behavior here would be to use the hover style to signal the keyboard position when users navigate through the menu items (black text on gray bg). The active state (blue on blue) should only be triggered when an item is clicked/tapped on or activated via keyboard.
The selected state (black text on blue bg) does not actually apply to the TahS component, since it's used to indicate the current selection (e.g., made in a Select, Lookup or Combobox component).

@Sarai-WMDE thanks for your quick reply!

Re: TypeaheadSearch keyboard nav, I will work on implementing these changes.

For the other two, I started implementing props for these things, but ran into an issue that also affects the showThumbnail prop that I hadn't considered before: if we want to make these options available to any component that has a menu, we have to include them as props on those components, too. This means a lot of repeated code and extra props. I wanted to run some potential solutions by you, @egardner, and @Catrope.

My first thought was to do one of the following things for each available menu item customization, depending on what it is:

1. Set a default value, then assume library users can customize it via CSS. This would work for things like bolding the label or controlling the text overflow behavior. However, it would be nice to provide props for common customizations like this.
2. Attempt to calculate the most ideal value (like bolding the label if there is a description or making showThumbnail true if any menu item has a thumbnail), but that gets complicated and imprecise quickly, and explicit props are preferred.

We could include showThumbnail, boldLabel, and hideTextOverflow as props on every menu component, which will get passed into Menu, then into MenuItem. This is a lot of extra props and prop drilling, and if we add more flexibility to the MenuItem component in the future, we'd have to add the prop to every menu component.

I think the most ideal solution is to create a menuConfig prop and add it to any component that contains a menu. It would have a TypeScript type to define the shape and look something like this:

interface MenuConfig {
    showThumbnail: boolean,
    boldLabel: boolean,
    hideTextOverflow: boolean
}

This could get passed to the Menu component via v-bind="menuConfig, which would spread out the properties into separate props from there.

I think this would allow us to keep things as concise and future-proof as possible. Would love to hear others' thoughts!

Change 769111 merged by jenkins-bot:

[design/codex@main] MenuItem: Merge in ListTile and reflect updated designs

https://gerrit.wikimedia.org/r/769111

Change 769114 merged by jenkins-bot:

[design/codex@main] docs: Improve demos of components that use menus

https://gerrit.wikimedia.org/r/769114

In T300625#7765095, @AnneT wrote:

[...]
I think the most ideal solution is to create a menuConfig prop and add it to any component that contains a menu. It would have a TypeScript type to define the shape and look something like this:

interface MenuConfig {
    showThumbnail: boolean,
    boldLabel: boolean,
    hideTextOverflow: boolean
}

This could get passed to the Menu component via v-bind="menuConfig, which would spread out the properties into separate props from there.

I think this would allow us to keep things as concise and future-proof as possible. Would love to hear others' thoughts!

This sounds like a good solution to me. If possible, it might be a good idea to think about a default value for each of these config options. That way, one could make each option optional and even the menuConfig prop itself wouldn't need to be required.
That being said, it would be fine to start out with the prop and each config option being required. They can be made optional later on, when we have more experience with what commonly used values are. Making these optional is a non-breaking change, so it doesn't have to be made now.

Thanks for the feedback, @Michael! I definitely think we can make all of the properties in the menu config optional—they should all default to false, like individual boolean props, and many instances of menu components will stick to those default values.

I like the MenuConfig approach, with defaults for all keys as Michael said. As for the selection behavior change that Sarai mentioned, I think you could just disable the select-on-highlight feature, and ignore update:selected events from the menu (or at least, don't actually update the selection in response to them, but navigate to the option's href instead).

Change 770014 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[design/codex@main] Menu, MenuItem: Add menuConfig, enable boldLabel & hideDescriptionOverflow

https://gerrit.wikimedia.org/r/770014

@Catrope awesome, I've got a patch up adding the MenuConfig type and prop!

Re: the selection behavior, the reason we're selecting the menu item on keyboard highlight is so that, if the user hits enter, we have a way of telling, in the onSubmit handler, which item was selected for instrumentation purposes. We can't rely on the highlighted item because it's cleared when the menu is closed.

I've opened T303631, perhaps we can discuss this more there?

Here's the demo site instance for the latest patch, which enables bolding the label and hiding the description overflow via props, and adds the MenuConfig type and menuConfig object to components containing menus.

Change 770014 merged by jenkins-bot:

[design/codex@main] Menu, MenuItem: Add menuConfig, enable boldLabel & hideDescriptionOverflow

https://gerrit.wikimedia.org/r/770014

Moving this to design review because nearly all elements of the spec have been implemented. Some outstanding items:

• Option groups, including option group headers, are not yet supported or styled. I would recommend we add a separate task for this and complete it later.
• The ability to add HTML to menu item label and description. As I mentioned in the figma file, this would be difficult to offer, and I question whether it's needed for MenuItems (vs. something like the Card component, which is more of a visual component and doesn't live within a menu, for which we could allow users to add any content to the label and description so they can have rich text)

Thanks, Anne! Answering inline...

• Option groups, including option group headers, are not yet supported or styled. I would recommend we add a separate task for this and complete it later.

That's perfectly fine. I should have made it clear in the specs that headers are not necessarily part of the scope for this component at the moment. I created the needed separate task: T305036

• The ability to add HTML to menu item label and description. As I mentioned in the figma file, this would be difficult to offer, and I question whether it's needed for MenuItems (vs. something like the Card component, which is more of a visual component and doesn't live within a menu, for which we could allow users to add any content to the label and description so they can have rich text)

Roger that. The use case that implied being able to edit the format of the label and description text was removed from the specs as a result of our conversation in Figma. There doesn't seem to be a need for formatting said content for now (we got a thumbs up for removing italics – currently used in Wikidata search suggestions – from product over at WMDE).

Some comments after reviewing the design of the MenuItem component in its demo page:

Component design

• (Not fixable now. Requires 'tokenization') The font-size and line-height of this (and all other) Codex components don't match the size specified in the designs. This doesn't require a code update: the misalignment was already detected and recorded as an internal design discussion/task. Just mentioning this here for alignment purposes.

• It looks like this (and all other) component is inheriting (?) a legacy font stack. The font-family of all components should be defined by the token @font-family-base

• The size of the thumbnail and icon containers (currently defined by @size-search-figure) should ideally be 2.5em x 2.5em. A quick fix for now would be replacing 36 by 35 in the existing calc (?). We could also wait and potentially use spacing tokens instead of that calculation, since this is quite a small detail.

Demo and visualization

• It would be great if the 'Default' component demo also displayed the active and selected state (which would be removed by clicking outside the component). What do you think? Just realized this is not technically possible, as indicated in the demo page.

• (Not blocking) It would be really helpful if we added some configuration to the Default MenuItem, so users could try out its props: such as the ability to make it disabled, editing its label and description, removing or selecting a different icon (like with Button), removing or adding a thumbnail, and try out hideDescriptionOverflow.

In case we agreed to add this config table, then I would suggest creating a separate task.

@Sarai-WMDE thanks for all the feedback! Some comments...

In T300625#7828911, @Sarai-WMDE wrote:

• (Not fixable now. Requires 'tokenization') The font-size and line-height of this (and all other) Codex components don't match the size specified in the designs. This doesn't require a code update: the misalignment was already detected and recorded as an internal design discussion/task. Just mentioning this here for alignment purposes.

Could you share a link to the task on font-size and line-height? Some of us were talking about this in code review recently and wondering especially about the font-size, and whether a 14px base font size is a MediaWiki-specific thing or not. We've chosen to keep everything at a 16px base font size until we have clarity on that (although we should be able to align on line height now since it's just a ratio)

• It looks like this (and all other) component is inheriting (?) a legacy font stack. The font-family of all components should be defined by the token @font-family-base

This should be an easy fix; the font-family styles for the VitePress site are manually set and not using design tokens like they should be. Is there already a task for this? If not I can open one and resolve it quickly!

The size of the thumbnail and icon containers (currently defined by @size-search-figure) should ideally be 2.5em x 2.5em. A quick fix for now would be replacing 36 by 35 in the existing calc (?). We could also wait and potentially use spacing tokens instead of that calculation, since this is quite a small detail.

I'll look into this!

(Not blocking) It would be really helpful if we added some configuration to the Default MenuItem, so users could try out its props: such as the ability to make it disabled, editing its label and description, removing or selecting a different icon (like with Button), removing or adding a thumbnail, and try out hideDescriptionOverflow.

Totally agree; I'll add a task for this so we don't forget.

• (Not fixable now. Requires 'tokenization') The font-size and line-height of this (and all other) Codex components don't match the size specified in the designs. This doesn't require a code update: the misalignment was already detected and recorded as an internal design discussion/task. Just mentioning this here for alignment purposes.

Could you share a link to the task on font-size and line-height? Some of us were talking about this in code review recently and wondering especially about the font-size, and whether a 14px base font size is a MediaWiki-specific thing or not. We've chosen to keep everything at a 16px base font size until we have clarity on that (although we should be able to align on line height now since it's just a ratio)

I guess that task would be Define the system's typographic styles and scale. Unfortunately, that exploration (the pairing of font sizes with line heights to create a typographic scale) is on a very early stage and hasn't been reviewed by the rest of the designer yet. (Hopefully that'll happen this week).

• It looks like this (and all other) component is inheriting (?) a legacy font stack. The font-family of all components should be defined by the token @font-family-base

This should be an easy fix; the font-family styles for the VitePress site are manually set and not using design tokens like they should be. Is there already a task for this? If not I can open one and resolve it quickly!

There's no task yet. I'll create it tomorrow morning (CEST) in case you haven't jumped ahead :)

The size of the thumbnail and icon containers (currently defined by @size-search-figure) should ideally be 2.5em x 2.5em. A quick fix for now would be replacing 36 by 35 in the existing calc (?). We could also wait and potentially use spacing tokens instead of that calculation, since this is quite a small detail.

I'll look into this!

I just realized that my suggestion's not right. Adjusting the calculation would correct the thumbnail size in the Codex component, but it'd make it become smaller in context due to the compounding effect (16px v. 14px component font size in Vector) :-/

(Not blocking) It would be really helpful if we added some configuration to the Default MenuItem, so users could try out its props: such as the ability to make it disabled, editing its label and description, removing or selecting a different icon (like with Button), removing or adding a thumbnail, and try out hideDescriptionOverflow.

Totally agree; I'll add a task for this so we don't forget.

That's great. Thanks! (For everything)

I've created a task for fixing the font family!

The size of the thumbnail and icon containers (currently defined by @size-search-figure) should ideally be 2.5em x 2.5em. A quick fix for now would be replacing 36 by 35 in the existing calc (?). We could also wait and potentially use spacing tokens instead of that calculation, since this is quite a small detail.

I just realized that my suggestion's not right. Adjusting the calculation would correct the thumbnail size in the Codex component, but it'd make it become smaller in context due to the compounding effect (16px v. 14px component font size in Vector) :-/

Yeahhh it seems like this difference in base font size is hindering us...would it be enough of a temporary solution for us to set the demos on the docs site to have a 14px font size? This way, we wouldn't be including that base font size in the components themselves, but the demos would look the same as the designs and might make design-developer handoff and these kinds of calculations easier.

In T300625#7830054, @AnneT wrote:

I've created a task for fixing the font family!

Thank you!

I just realized that my suggestion's not right. Adjusting the calculation would correct the thumbnail size in the Codex component, but it'd make it become smaller in context due to the compounding effect (16px v. 14px component font size in Vector) :-/

Yeahhh it seems like this difference in base font size is hindering us...would it be enough of a temporary solution for us to set the demos on the docs site to have a 14px font size? This way, we wouldn't be including that base font size in the components themselves, but the demos would look the same as the designs and might make design-developer handoff and these kinds of calculations easier.

That'd work, yes. It's probably what we'll end up doing. But let me get back to you, I'll like to ping the pattern lab regarding font sizes first, if that's ok.

⚠ ⚠ ⚠ I completely forgot to mention a couple of needed changes affeccting the thumbnail element:

1. In the current implementation, it has a box-shadow applied to it that should be removed and replaced by a 1px border (@border-width-base) with the color #C8CCD1 (Base70). This shade is currently documented as @border-color-base--disabled in codex-base.json. But this token's name will most likely change (we have documented it with a different nomenclature in Figma). I'm not sure of what the best approach would be here: if we used the existing border token, would we get a heads-up (reference error) once the token is modified? That could be helpful.

2. Also, the placeholder svg's color should be Base30/@color-placeholder. It's color also shouldn't change when the MenuItem is selected.

Change 789269 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[design/codex@main] MenuItem: Update thumbnail styles

https://gerrit.wikimedia.org/r/789269

Change 789269 merged by jenkins-bot:

[design/codex@main] MenuItem: Update thumbnail styles

https://gerrit.wikimedia.org/r/789269

Signing this off because all original requirements and requested fixes have been met or covered. Pending tasks have been documented in separate tickets (see subtasks).

Design signed off. Looks like we need QTE sign-off?

This has been already been reviewed and is good to go!

product sign-off completed