Page MenuHomePhabricator

Add supporting text in MenuItem component
Closed, ResolvedPublic

Description

Background

  • Description: Menu item component needs a supportingText prop.
  • History: describe or link to prior discussions related to this component
  • Known use case(s):
    • Supporting text will be used to indicate when search results are from redirects in T303013.
      Captura de Pantalla 2022-11-29 a las 11.21.40.png (448×900 px, 273 KB)
  • Considerations: list any known challenges or blockers, or any other important information

User stories

  • As a user I need to understand when a search result in a menu is from a redirect.

Previous implementations

Design spec

Open questions

Add here the questions to be answered in order to design and implement the component

Acceptance criteria (or Done)

Design

  • Design the Figma spec sheet and add it in this task
  • Update the component in the Figma library. This step will be done by a DST member.

Code

  • Add supportingText prop in the MenuItem

Event Timeline

@AnneT the Figma spec sheet for the menu item with supporting text is Ready for Development.

Supporting text will be added in the same title text line and they will use different colors; we'll just wrap them in separate <span> elements.

Captura de Pantalla 2022-11-29 a las 11.27.44.png (638×1 px, 265 KB)

@bmartinezcalvo @Sarai-WMDE well, I'm a dummy and forgot that we had a special prop for match because we needed to be able to apply a lang attribute to the match itself, which might be different from the language of the label. So we do need to keep the match prop.

That said, the match and supportingText items play pretty nicely together. Here's an example with "ali" as the search term:

image.png (146×520 px, 20 KB)

Let me know if you want to change anything, but I think this works fine and is unlikely to come up as an actual use case.

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

[design/codex@main] MenuItem: Add supportingText prop

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

AnneT changed the task status from Open to In Progress.Nov 29 2022, 3:53 PM

@bmartinezcalvo / @Sarai-WMDE I've got a patch up and was hoping you could take a look it. See the configurable demo for MenuItem.

This is an existing issue, but it's now potentially more noticeable since adding the supporting text: if the label, match, and/or supporting text are very long, the line of text will wrap. When this happens, the space between the lines is slightly bigger than the space between the title line and the description:

image.png (196×1 px, 39 KB)

This may even be more noticeable if we increase the line height of the text within the menu item (see T322383).

What do you think we should do? There is not currently an option for trimming the first line of text and adding ellipses like we do for the description.

This is an existing issue, but it's now potentially more noticeable since adding the supporting text: if the label, match, and/or supporting text are very long, the line of text will wrap. When this happens, the space between the lines is slightly bigger than the space between the title line and the description:

image.png (196×1 px, 39 KB)

All the text elements in the menu item seem to (and should) be using the same line-height, which should technically make everything evenly spaced 🤔 I believe that the unexpected additional line-height spacing might be caused by the parenthesis symbol.

What do you think we should do? There is not currently an option for trimming the first line of text and adding ellipses like we do for the description.

That's a possibility, but titles and their accompanying information are usually too relevant (strongly influence users' selection) to ever want to apply ellipsis to them. Another potential solution would be to add some small padding between the title and description by default. Nevertheless, although the issue is annoying, there are several factors that need to occur for it to appear, so I'd tend to leave it as is for now.

This may even be more noticeable if we increase the line height of the text within the menu item (see T322383).

The current line-height applied to the menu item elements in the demo (1.4285714) is a legacy value, and should be replaced by a new (and actually smaller) one. Typographic styles are almost done, but it is pending for designers to replace all the styles in our component library. While doing that, we should make sure to verify that the right values are applied in the Codex demo, in parallel. A problem that we'll encounter is that, in the current version of the font tokens, we've defined different line-height values for the desktop and mobile UI text sizes (14 and 16px), in order to keep them even in size (around 22px in both cases). This of course gets in the way of our current approach, which lets fonts and line-heights scale between the demo (which reflects mobile) and production. I wonder if we might want to start explicit about the fact that our demo is showcasing mobile styles, and begin to document web components too (which their own styles) in order to solve this inconsistency.

Thanks for the feedback @Sarai-WMDE! It makes sense that we shouldn't truncate the title, and seems like we can leave the spacing as-is at least for now.

I would love to be able to add a theme switcher to the demos so we could easily test out the different base font sizes!

That's a possibility, but titles and their accompanying information are usually too relevant (strongly influence users' selection) to ever want to apply ellipsis to them. Another potential solution would be to add some small padding between the title and description by default. Nevertheless, although the issue is annoying, there are several factors that need to occur for it to appear, so I'd tend to leave it as is for now.

As Sarai comments, we could add a small padding between title and description by default. If we add 2px separation between title and subtitle it would look better when the supporting text wrap into two or more lines.

Captura de Pantalla 2022-11-30 a las 16.10.32.png (478×1 px, 209 KB)

It would look good in any menu item with just title and description.

Captura de Pantalla 2022-11-30 a las 16.11.11.png (314×1 px, 85 KB)

Also the height of some components using menu items would grow but not a lot.

Captura de Pantalla 2022-11-30 a las 16.14.47.png (1×1 px, 686 KB)

@bmartinezcalvo makes sense to me—could we do this along with correcting the overall line height in T322383? It would be best to handle that all at the same time so we can let the Web team know about the change in menu item height.

@bmartinezcalvo to note: we've tried to ensure that the case where there is a title and description they are vertically centered with the thumbnail

Screen Shot 2022-11-30 at 1.41.39 PM.png (485×800 px, 209 KB)

Change 861887 merged by jenkins-bot:

[design/codex@main] MenuItem: Add supportingText prop

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

@bmartinezcalvo to note: we've tried to ensure that the case where there is a title and description they are vertically centered with the thumbnail

Screen Shot 2022-11-30 at 1.41.39 PM.png (485×800 px, 209 KB)

@alexhollender_WMF we should decide which is the priority here:

  1. Title and description being centered with the thumbnail but supporting text being so closed to the description when supporting text is 2 lines.
  2. Title and description being not exactly centered with the thumbnail but supporting text separation being always balanced.

The question is: will the supporting text be in a 2nd line in many cases? or is it just a rare case?

@AnneT design sign-off done for supporting text. It works well in Chrome, Safari and Firefox. Moving the task to QTE sign-off.

@bmartinezcalvo makes sense to me—could we do this along with correcting the overall line height in T322383? It would be best to handle that all at the same time so we can let the Web team know about the change in menu item height.

We can discuss about the padding between title and description and if we decide to increase it to 2px we can do it in T322383

@bmartinezcalvo to note: we've tried to ensure that the case where there is a title and description they are vertically centered with the thumbnail

@alexhollender_WMF we should decide which is the priority here:

  1. Title and description being centered with the thumbnail but supporting text being so closed to the description when supporting text is 2 lines.
  2. Title and description being not exactly centered with the thumbnail but supporting text separation being always balanced.

The question is: will the supporting text be in a 2nd line in many cases? or is it just a rare case?

@bmartinezcalvo can you include images to illustrate the two cases you mention? Also, maybe it's possible to adjust the spacing in cases where a description is present (so, in other words, we can have the best alignment in both cases).

Change 865151 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[mediawiki/core@master] Update Codex from v0.3.0 to v0.4.0

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

Change 865151 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.3.0 to v0.4.0

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

  1. The component states are as designed.
  2. The text overflow feature is as expected.
  3. RTL and LTR appear are as expected.
  4. Text font colours (description and supporting text) is as designed.

Tested and passed on Chrome v106, Safari v16 and Firefox v107