Page MenuHomePhabricator

Dialog: support header and footer customization
Closed, ResolvedPublic5 Estimated Story Points

Description

Background goal

The Dialog component should be updated to allow for some header and footer customization by the user:

Abstract Wikipedia

image.png (906×1 px, 322 KB)
Screenshot 2022-12-21 at 3.31.35 PM.png (468×966 px, 67 KB)
Header subtitleFooter text

Other use cases in production

Captura de Pantalla 2023-01-23 a las 17.05.00.png (488×1 px, 411 KB)
Footer text above the buttons

In other use cases, a checkbox is placed near the next or main button to check permanent actions like “Don’t show this again” or “Remember me”

Captura de Pantalla 2023-01-24 a las 10.00.49.png (986×956 px, 239 KB)

Design spec
Acceptance Criteria
  • Create a new Figma spec version for Dialog component with header subtitle and footer text and add it in this task
  • Update Dialog in the Figma library and publish it to be reusable
  • Implement Codex demo Dialog component with subtitle and footer properties
  • Update the DSG page with the new paddings and use cases

Design Review
  • footerText should use text size 14px
  • There should be 6px padding between Title and Subtitle
  • Padding between title, body and footer should be 32px
  • Subtitle text should be 16/22
  • Footer text should be 14/22

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

I think this is where we can have @KieranMcCann provide visual design guidance to revise white space of content above footers and headers so that it doesn't feel crowded without dividers. Having dividers only appear when there is scrolling content is makes sense for our design principle of "Content first" and using more decorative elements sparingly, which in this case is affordance to only show dividers when there is scrolling content.

@RHo assigning this task to @KieranMcCann to review the separation between Header-Body-Footer sections. I've prepared this collection with the most relevant/problematic use cases so you can have a look and decide the right separation. Once you've defined the best visual approach I will update the main component and the spec with the final solution.

I agree with avoiding the use of dividers as much as possible. But there are some specific cases where dividers are useful to visually separate the content blocks in the dialog, specially when the footer has text or permanent actions. In this case, the dialog without divider seems too crowded.

Captura de Pantalla 2023-02-01 a las 13.57.06.png (686×2 px, 655 KB)
Captura de Pantalla 2023-02-01 a las 13.56.49.png (678×2 px, 650 KB)
With dividersWithout dividers

@bmartinezcalvo Even in these examples I think the divider is probably not necessary and as @RHo pointed out the addition of some white space may help. Thanks for assigning the task to me. I’ll take a look and update later today.

Here’s an updated version where I’ve increased spacing below the header and above the footer by 8px. I think this does enough to separate the elements without the need for dividers. Let me know your thoughts @RHo and @bmartinezcalvo.

Screenshot 2023-02-02 at 12.14.14.png (934×4 px, 313 KB)

Here’s an updated version where I’ve increased spacing below the header and above the footer by 8px. I think this does enough to separate the elements without the need for dividers. Let me know your thoughts @RHo and @bmartinezcalvo.

Screenshot 2023-02-02 at 12.14.14.png (934×4 px, 313 KB)

Thanks @KieranMcCann, looks good to me. Could you also apply the same spacing treatment to the header when there is a subtitle as well for completion?

Thanks @KieranMcCann, looks good to me. Could you also apply the same spacing treatment to the header when there is a subtitle as well for completion?

That’s done.

dialog-no-dividers-suggested-spacing.png (672×3 px, 93 KB)

Here’s the Figma artboard if you want a closer look.

If you’re both happy with this, shall I update all the components and documentation in the Figma file @bmartinezcalvo?

That’s done.

dialog-no-dividers-suggested-spacing.png (672×3 px, 93 KB)

Here’s the Figma artboard if you want a closer look.

@KieranMcCann it LGTM. As a result of this new padding, the dialog sections are visually separated without the use of dividers. I have just 2 questions about this new design (also added as Figma comments):

  1. Should we differentiate the Body text from the Footer one? They are the same text size on desktop. I used color-subtle to represent the footer text differently from the body one but not sure if it's enough.
  2. Do we want to maintain the same paddings here? Or do we want these fixed sections to be shorter when scrolled?

If you’re both happy with this, shall I update all the components and documentation in the Figma file @bmartinezcalvo?

@KieranMcCann sure, feel free to create a new version in the Figma file with the final decision.

  1. Should we differentiate the Body text from the Footer one? They are the same text size on desktop. I used color-subtle to represent the footer text differently from the body one but not sure if it's enough.

I think that could work as new default. Keep in mind that we need contextually a different treatment in a place like VisualEditor in order to be contrasty enough.

  1. Should we differentiate the Body text from the Footer one? They are the same text size on desktop. I used color-subtle to represent the footer text differently from the body one but not sure if it's enough.

I think that could work as new default. Keep in mind that we need contextually a different treatment in a place like VisualEditor in order to be contrasty enough.

@Volker_E the problem to do that is that, as we are using text size 14px for the body on desktop, our next smaller text size is 12px x-small which I guess would be too small for accessibility reasons. On mobile we can use 14px for footer text since we are using 16px for the body. Check the spec sheet. Let me know if 12px would be too small for this case or if it's ok to use this text size for this terms and conditions text (they are usually smaller than body text).

@bmartinezcalvo I added comments in Figma but just putting them here too for visibility.

  1. Should we differentiate the Body text from the Footer one? They are the same text size on desktop. I used color-subtle to represent the footer text differently from the body one but not sure if it's enough.

I agree that I’d rather not use 12px text as a rule. This is a bigger question but I wonder if the issue is that our color-subtle is actually too similar to our base colour? I appreciate we need to meet accessibility contrast standards but I do find them very similar. Assuming that’s not an option though, I think the current iteration is fine and doesn’t require a smaller font size. The footer text should be relevant information, that users need to read, otherwise it shouldn’t be there, so I don’t think we need to reduce its visual hierarchy any more than we are.

  1. Do we want to maintain the same paddings here? Or do we want these fixed sections to be shorter when scrolled?

When we have the title with the larger padding below and then the divider line, it does look a little off-centre...

Screenshot 2023-02-07 at 10.20.03.png (400×1 px, 100 KB)

So yes I think we could reduce that padding when the divider line is there unless that was to cause any issues.

Picking this task to update the spec sheet with the last decisions taken about paddings and footer text.

I've created this new Figma spec sheet version with the visual improvements proposed by Kieran:

  • Removed dividers property by default (dividers will be visible just when scrolling).
  • Used 32px as separation between Header, Body and Footer. This padding will be 16px when scrolling so we save space in the header and footer
  • Header subtitle will be collapsed when scrolling to save space.
  • Footer text: 14px size and color-subtle on both desktop and mobile. On desktop the body and footer texts will be the same size (unless we increase the base font to 16px). On mobile, as we use 16px base font the body text will be 16px and the footer text will be 14px, so the difference between both texts will be more obvious.

@KieranMcCann @RHo if you both agree and the new design is ok we can move the task to Ready for development.

Great! Moving the task to Ready for development then.

Now that this task is ready for development again, I have some questions before resuming implementation work here.

It looks like there are now two different optional footer elements that the Dialog component can display (in addition to the buttons):

  • Optional Footer (small, grey) which always appears above the Dialog buttons
  • Something you are calling a "Permanent Action" which is aligned with the buttons if space allows. In the specs this is either an icon-only button or a checkbox with a label.

Question 1: Can footer text show up independently of the Dialog buttons? If a dialog has no buttons ("submit", "cancel", etc) then should it be impossible to display this grey text in the footer area?
Question 2: Can the "permanent action" area contain arbitrary content from the user? I'm assuming so because the two examples are pretty different (icon-only button vs checkbox with label). Should this content remain visible even if there are no buttons to display in the footer?
Question 3: What happens if the user provides footer text and "primary action" content but no buttons? Is the visibility of these elements dependent on the presence of the primary/default footer buttons?
Question 4: If the user can provide rich text (links, etc) in the footer text region, then they can also provide basically anything else. Is this ok? The way that a user would include a link inside this text is by providing the markup into an open-ended <slot> in the component. But this also opens the door up for other arbitrary content (icons, paragraphs, images, etc). The design guidelines can recommend against this, but it will be possible. If this is not acceptable, we have two options. We can either change the "footer" element so that it can only accept plain text (no links or formatted text), or we make the component significantly more complex so that it displays plain text and links but discards other markup that the user may have provided to it in error. I would recommend against the latter approach unless the ability to include links in this text is really important for some use-cases we intend to support (legal disclaimer or something).

Also as an aside, I am hesitant to use the term "permanent action" to describe this new content region in the footer. We already have DialogAction and PrimaryDialogAction in our code (for the Dialog buttons themselves). I think we should come up with some other term to describe this new area. OptionalFooterContent or FooterInfo maybe?

egardner renamed this task from Dialog: slots for Dialog headers and footers to Dialog: support header and footer customization.Feb 15 2023, 11:20 PM
egardner updated the task description. (Show Details)

Question 1: Can footer text show up independently of the Dialog buttons? If a dialog has no buttons ("submit", "cancel", etc) then should it be impossible to display this grey text in the footer area?

@egardner footer text has been designed as part of the buttons section since this informative text always refers to the buttons (e.g. "by clicking publish you will..."). So I think the text should always be visible when buttons are displayed. Do we have any other use case where we want to display just a footer text without buttons?

Question 2: Can the "permanent action" area contain arbitrary content from the user? I'm assuming so because the two examples are pretty different (icon-only button vs checkbox with label). Should this content remain visible even if there are no buttons to display in the footer?

Permanent action refer to the entire dialog, so in this case I think it could be visible if there is no buttons. (Added this and the previous use case as recommendations in the spec).

Captura de Pantalla 2023-02-17 a las 9.22.30.png (734×2 px, 502 KB)

Question 3: What happens if the user provides footer text and "primary action" content but no buttons? Is the visibility of these elements dependent on the presence of the primary/default footer buttons?

Do you mean something like this where the main button is within the body content instead? I think this case would not be possible since dialog buttons should be at the end when they affect to the entire dialog. We could add a button within the text if it refers just to the content or to a section in the content. Footer text will always be related with the footer buttons.

Captura de Pantalla 2023-02-17 a las 9.24.46.png (772×1 px, 353 KB)

Question 4: If the user can provide rich text (links, etc) in the footer text region, then they can also provide basically anything else. Is this ok? The way that a user would include a link inside this text is by providing the markup into an open-ended <slot> in the component. But this also opens the door up for other arbitrary content (icons, paragraphs, images, etc). The design guidelines can recommend against this, but it will be possible. If this is not acceptable, we have two options. We can either change the "footer" element so that it can only accept plain text (no links or formatted text), or we make the component significantly more complex so that it displays plain text and links but discards other markup that the user may have provided to it in error. I would recommend against the latter approach unless the ability to include links in this text is really important for some use-cases we intend to support (legal disclaimer or something).

The footer text will need to display links in some cases since the use cases we want to cover need to display the Terms link within the text. I can provide more recommendations about the use of the footer, but the idea is that this text just provides text and links.

Also as an aside, I am hesitant to use the term "permanent action" to describe this new content region in the footer. We already have DialogAction and PrimaryDialogAction in our code (for the Dialog buttons themselves). I think we should come up with some other term to describe this new area. OptionalFooterContent or FooterInfo maybe?

I named it PermanentAction since this action will be performed once and will affect the entire dialog. For example, if the user selects the "Don't show again" this action will be permanent over time and the dialog information will no longer be displayed. Not sure if OptionalFooterContent or FooterInfo are good names for this action.

Is Codex Dialog component meant to be the only dialog component in the library? (I can only find one Dialog in Codex_Planned_Components)

It seems the component is a good replacement for OOUI's SimpleDialog but it's not covering OOUIs ProcessDialog structure. Since this task is about the header and footer customization; I wonder if ProcessDialog use cases should be taken in account. One example of widespread usage of them is VisualEditor's save dialog. Also in the GrowthExperiments extension:

Screenshot 2023-02-17 at 14.40.23.png (720×1 px, 131 KB)
Screenshot 2023-02-17 at 14.45.02.png (1×1 px, 183 KB)
Visual editor save dialogGrowth experiments task type dialog

Could you clarify if there are plans to support ProcessDialog-like customization where actions or arbitrary content could be placed in the header/footer?


Could you clarify if there are plans to support ProcessDialog-like customization where actions or arbitrary content could be placed in the header/footer?

@Sgs there has been some discussion around whether we want to also include a "process-dialog" style component in Codex here: T321893: Evaluate fullscreen mobile Dialog in Codex – this discussion is also kind of wrapped up with how "large" / full-screen dialogs should work (on both desktop and mobile). I'm not 100% sure what the verdict here is, but if you have specific use-cases that you need that might help to resolve things.

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

[design/codex@main] [WIP] Dialog: support header & footer customization

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

@egardner I've checked your last patch on Dialog and I think we could improve the Configurable Demo with the following:

  • closeButtonLabel: do we really need this prop where the user can write? I think the right configurable property here should be "show/hide" using a toggle switch
  • DefaultAction: since we are using PrimaryAction, could we rename this to SecondaryAction?
    • Also, could we change its copy from "Close dialog" to "Cancel"? Since we already have the close button on top, I would avoid displaying two different close options in the same dialog demo
  • default slot: could it be renamed to “body”?
  • I know that we always use the code nomenclature to represent our components and properties, but in some cases it's not easy to read (e.g. primaryActionDisabled). Is there any chance to update it and separate each word (e.g. "Primary Action Disabled")?

Apart from this, I've checked that for some reason the Dialog title is now Regular and it should be Bold.

egardner changed the point value for this task from 3 to 5.Mar 6 2023, 11:55 PM

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

[design/codex@main] docs: Unify implementation of Dialog slots in configurable demo

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

@egardner heads up that in this new iteration of the dialog we proposed to use the dividers just when the dialog is scrollable. So showDividers should be removed from the component's properties.

Also, footer text should use text size 14px (in both desktop and mobile) since we want these texts to be smaller than the body one.

Change 893808 merged by jenkins-bot:

[design/codex@main] Dialog: support header & footer customization

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

Design sign-off done.

@egardner adding here some things to fix:

  1. We proposed to use the dividers just when the dialog is scrollable. So showDividers should be removed from the component's properties and dividers should be displayed just when the dialog content is too long and it's scrollable.
  2. footerText should use text size 14px (in both desktop and mobile) since we want these texts to be smaller than the body one.
  3. There should be 6px padding between Title and Subtitle.
  4. Since dividers will be visible just when scrolling, we increased the padding between title, body and footer to 32px.
  5. When scrolling, subtitle should be hidden so the header doesn't occupy to much space.
  6. Regarding the configurable properties, could we rename Slot > default to Slot > body?

@egardner moving to Ready for development to fix the items listed above.

Design sign-off done.

@egardner adding here some things to fix:

  1. We proposed to use the dividers just when the dialog is scrollable. So showDividers should be removed from the component's properties and dividers should be displayed just when the dialog content is too long and it's scrollable.
  2. footerText should use text size 14px (in both desktop and mobile) since we want these texts to be smaller than the body one.
  3. There should be 6px padding between Title and Subtitle.
  4. Since dividers will be visible just when scrolling, we increased the padding between title, body and footer to 32px.
  5. When scrolling, subtitle should be hidden so the header doesn't occupy to much space.
  6. Regarding the configurable properties, could we rename Slot > default to Slot > body?

Hey @bmartinezcalvo, since we are planning to cut another Codex release today, I'm actually going to just revert all the work that has been done here so far – I don't want any code going out in a release that is not aligned with the design specs.

That being said, I think that the following items go beyond the scope of this task:

  1. We proposed to use the dividers just when the dialog is scrollable. So showDividers should be removed from the component's properties and dividers should be displayed just when the dialog content is too long and it's scrollable.
  2. Since dividers will be visible just when scrolling, we increased the padding between title, body and footer to 32px.
  3. When scrolling, subtitle should be hidden so the header doesn't occupy to much space.

I recommend filing a separate stand-alone task for proposed changes Dialog scrolling behavior (some of which might be complicated to implement).

Additionally, the slot names in the configurable demo are meant to match the actual property names in the component code. In Vue, default is always the name for the main slot and this is something we don't want to change here.

So that leaves the following fixes as within scope for this task:

  1. footerText should use text size 14px (in both desktop and mobile) since we want these texts to be smaller than the body one.
  2. There should be 6px padding between Title and Subtitle.

I can address these once I re-introduce this patch after today's Codex release.

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

[design/codex@main] Revert "Dialog: support header & footer customization"

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

Change 898850 merged by jenkins-bot:

[design/codex@main] Revert "Dialog: support header & footer customization"

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

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

[mediawiki/core@master] Update Codex from v0.6.2 to v0.7.0

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

Hey @bmartinezcalvo, since we are planning to cut another Codex release today, I'm actually going to just revert all the work that has been done here so far – I don't want any code going out in a release that is not aligned with the design specs.

That being said, I think that the following items go beyond the scope of this task:

  1. We proposed to use the dividers just when the dialog is scrollable. So showDividers should be removed from the component's properties and dividers should be displayed just when the dialog content is too long and it's scrollable.
  2. Since dividers will be visible just when scrolling, we increased the padding between title, body and footer to 32px.
  3. When scrolling, subtitle should be hidden so the header doesn't occupy to much space.

I recommend filing a separate stand-alone task for proposed changes Dialog scrolling behavior (some of which might be complicated to implement).

@egardner created this other task T332124 to fix all the items related to the dialog's scroll behavior there.

So that leaves the following fixes as within scope for this task:

  1. footerText should use text size 14px (in both desktop and mobile) since we want these texts to be smaller than the body one.
  2. There should be 6px padding between Title and Subtitle.

I can address these once I re-introduce this patch after today's Codex release.

@egardner we should also increase the padding between the title, body and footer to 32px in this task, since this is not part of the scroll behavior.

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

[design/codex@main] [WIP] Re-introduce Dialog header and footer customization

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

Change 896180 abandoned by Anne Tomasevich:

[design/codex@main] docs: Unify implementation of Dialog slots in configurable demo

Reason:

no longer needed

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

I've re-introduced this patch with some further improvements.

The Dialog now includes header and footer slots which by default include content that should line up with @bmartinezcalvo's design specs: optional title, subtitle, and close button in the header, optional action buttons and footer text in the footer. But both of these regions can also accept custom markup provided by the user which will completely replace the default content. This opens the way for other types of Dialogs that look and work differently – multi-stage process dialogs, a full-bleed header Onboarding Dialog, footers with "don't show again" checkboxes, etc. Developers can customize the header and footer content while keeping the rest of the component's behavior.

For something like a multi-step "Process Dialog", the best path forward would probably be to develop a new component that *wraps* the base Dialog, and pre-populates the customized header and footer options while also providing some concept of "steps" that the end-user can provide in sequence. But there may also be cases where a unique custom dialog is necessary, and now that will be possible as well.

You can see the staging docs page here (there is a new custom example at the end): https://900456--wikimedia-codex.netlify.app/components/demos/dialog.html
The "sandbox" page (not intended for use as public documentation) may also be useful for design review – this page includes some additional variations that are technically possible even if they are not part of the design guidelines: https://900456--wikimedia-codex.netlify.app/sandbox/#cdx-dialog.

I've tried to also address @bmartinezcalvo's latest design comments in terms of: spacing between title and subtitle, spacing between header / body / footer regions, and footer text size.

Finally, I have removed the showDividers prop in anticipation of some changes to the design spec in terms of how we want this to work. Going further than that is out of scope for this patch.

Change 900456 merged by jenkins-bot:

[design/codex@main] Re-introduce Dialog header and footer customization

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

I've re-introduced this patch with some further improvements.

The Dialog now includes header and footer slots which by default include content that should line up with @bmartinezcalvo's design specs: optional title, subtitle, and close button in the header, optional action buttons and footer text in the footer.

@egardner just checking you last patch for Dialog slots and wondering if closeButtonLabel property with the text input is needed in the configurable demo, since when I write "Skip" instead it doesn't display any difference in the close button. Not sure how we could show this property since we will have cases where a text will be displayed to skip the dialog (e.g. Onboarding Dialog).

Also, the separation betwen Header, Body and Footer should be 32px and now it seems bigger.

Change 903778 had a related patch set uploaded (by Catrope; author: Catrope):

[mediawiki/core@master] Update Codex from v0.7.0 to v0.8.0

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

Change 903778 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.7.0 to v0.8.0

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

@egardner just checking you last patch for Dialog slots and wondering if closeButtonLabel property with the text input is needed in the configurable demo, since when I write "Skip" instead it doesn't display any difference in the close button. Not sure how we could show this property since we will have cases where a text will be displayed to skip the dialog (e.g. Onboarding Dialog).

Also, the separation betwen Header, Body and Footer should be 32px and now it seems bigger.

@egardner design sign-off done. As commented yesterday, separation between Header, Body and Footer should be 32px since now it's bigger. Also, I just checked that the subtitle text it's not a body so it should be 16/22 instead 16/25.6. Same for the footer text, it should be 14/22 instead 14/22.4.

Apart from this, I would change a couple of things in the demo:

  1. The topic related to closeButtonLabel commented above
  2. I would rename the secondary button in the configurable demo from "Close dialog" to "Cancel". Since we already have the close button on top, I would avoid to reference to other close button.

Also, the separation betwen Header, Body and Footer should be 32px and now it seems bigger.

I agree that the spacing seems large, but I defined a 32px gap per the design specs. If you view the demo in Safari you can use the flex inspector to see (for some reason the one in Firefox doesn't work well due to the fact that the dialog as a whole is absolutely positioned on the page). The areas with the diagonal stripes are the flex gaps.

Screenshot 2023-03-29 at 9.13.52 AM.png (1×1 px, 393 KB)

Screenshot 2023-03-29 at 9.15.59 AM.png (276×504 px, 49 KB)

I wonder if this has to do with differences in terms of how Figma aligns objects vs how CSS does it. I even made sure to zero-out all padding and margins for the first and last content elements in the main dialog area.

If the attached images do not represent the visual result you intended, I think we should just reduce the gap spacing from 32px to a smaller value (24px maybe)?

for the footer text, it should be 14/22 instead 14/22.4.

Unless I'm looking at the wrong thing, the latest Figma spec still uses 16/20 for both subtitle and footer. I can update the values to the ones specified here in a follow-up patch shortly.

Regarding line-height, our line-height tokens are relative even though the design spec uses absolute values. It looks like this means the footer text needs @line-height-small while the subtitle needs @line-height-xx-small. In the future it would be helpful to see the appropriate font-size and line-height tokens referenced in the spec instead of just the final px value.

Also, the separation betwen Header, Body and Footer should be 32px and now it seems bigger.

I agree that the spacing seems large, but I defined a 32px gap per the design specs. If you view the demo in Safari you can use the flex inspector to see (for some reason the one in Firefox doesn't work well due to the fact that the dialog as a whole is absolutely positioned on the page). The areas with the diagonal stripes are the flex gaps.

Screenshot 2023-03-29 at 9.13.52 AM.png (1×1 px, 393 KB)

Screenshot 2023-03-29 at 9.15.59 AM.png (276×504 px, 49 KB)

I wonder if this has to do with differences in terms of how Figma aligns objects vs how CSS does it. I even made sure to zero-out all padding and margins for the first and last content elements in the main dialog area.

If the attached images do not represent the visual result you intended, I think we should just reduce the gap spacing from 32px to a smaller value (24px maybe)?

@egardner I'm comparing the Dialog demo with the one from design and both match exactly, so don't worry about the 32px, it seemed bigger to me at first glance but it's ok.

for the footer text, it should be 14/22 instead 14/22.4.

Unless I'm looking at the wrong thing, the latest Figma spec still uses 16/20 for both subtitle and footer. I can update the values to the ones specified here in a follow-up patch shortly.

Since Codex uses 16px as base font you should look at the mobile specs instead that are the ones that matches with the Codex demo font sizes:

Captura de Pantalla 2023-03-30 a las 11.38.21.png (772×1 px, 370 KB)

Subtitle here is 16/22 while Footer text is 14/22 becuase it will be used for small informative texts like the one for terms and conditions.

Regarding line-height, our line-height tokens are relative even though the design spec uses absolute values. It looks like this means the footer text needs @line-height-small while the subtitle needs @line-height-xx-small. In the future it would be helpful to see the appropriate font-size and line-height tokens referenced in the spec instead of just the final px value.

You are right, these are the right tokens we use for line-height. We will use text tokens instead in future specs. Thank you Eric!

@egardner moving this task to Ready for development so you can fix the subtitle and footer text line height.

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

[design/codex@main] Dialog: Ensure correct line-height is used in subtitle, footer

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

Change 907532 merged by jenkins-bot:

[design/codex@main] Dialog: Ensure correct line-height is used in subtitle, footer

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

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

[mediawiki/core@master] Update Codex from v0.8.0 to v0.9.0

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

Change 907988 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.8.0 to v0.9.0

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

Test wiki created on Patch demo by KHarlan (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/cfdfa13682/w

Test wiki on Patch demo by KHarlan (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/cfdfa13682/w/