Page MenuHomePhabricator

Font changes for QuickSurveys
Closed, ResolvedPublic2 Estimated Story Points

Description

In order to align QS survey design to the Wikimedia design style guide we need to implement the following font changes:

General Internal survey

Designs

CurrentDesiredSpecs
image.png (375ร—600 px, 30 KB)
001.jpg (432ร—600 px, 68 KB)
320.jpg (674ร—623 px, 109 KB)

Notes

  • Text color: Base 10 #202122.
  • Link color: Accent 50 #3366cc.
  • Font family: system fonts.
  • Privacy policy: Set the type to 13px. Line height to 18px.
  • Privacy policy: Set the text color to Accent 50 #3366cc.
Internal survey w/description and freeform text

Designs

CurrentDesiredSpecs
image.png (669ร—600 px, 50 KB)
image.png (217ร—220 px, 26 KB)
image.png (220ร—175 px, 28 KB)

Notes

  • All the above +
  • Type set to 14px. Line height to 20px.
  • Placeholder text color Base 30 #72777d.
  • Text color Base 10 #202122.
  • Button: Type set to 14px.
  • Button: Text color set to Accent 50 #3366cc.
Internal multiple answer survey

Designs

CurrentDesiredSpecs
image.png (463ร—600 px, 34 KB)
200.jpg (440ร—600 px, 63 KB)
324.jpg (440ร—776 px, 76 KB)

Notes

  • All the above +
  • Checkbox label Type set to 14px Line height 20px.
External survey

Designs

CurrentDesiredSpecs
image.png (454ร—600 px, 63 KB)
300.jpg (448ร—600 px, 113 KB)
325.jpg (469ร—600 px, 116 KB)

Notes

  • All the above
  • Description Type set to 14px. Line height 20px.
  • Font weight 400

QA

This patch will be reviewed by Design instead

Event Timeline

Madalina created this task.
Jhernandez changed the task status from Open to In Progress.Fri, Nov 5, 10:48 AM
Jhernandez claimed this task.

@aminalhazwani Re: footer line height, you wrote 13/18 which is ~1.38.

In the style guide variables there are two variables:

@line-height-base:                1.6;
@line-height-heading:             1.3;

With the normal 1.6 line height, the footer gets a 20.8px line height. Do you suggest we hardcode a 1.38 line height, keep the base line height, or use the line-height-heading?

Re: Link color, the skin defines the color for links, in the case of external links in Vector color: #36b;. A bit different from the specified link color: Accent 50 #3366cc.

Other link colors in vector are color: #0645ad; which is also different, but skin specific.

Mediawiki defines a set of base variables mediawiki.skin.defaults.less, and skins can override them, like Vector here mediawiki.skin.variables.less. These are where the link color is coming from.

If we specifically set the link color here, it will be the only link in the page with that color, that doesn't seem desirable which is why I'm checking. What should we do?

@aminalhazwani Re: footer line height, you wrote 13/18 which is ~1.38.

In the style guide variables there are two variables:

@line-height-base:                1.6;
@line-height-heading:             1.3;

With the normal 1.6 line height, the footer gets a 20.8px line height. Do you suggest we hardcode a 1.38 line height, keep the base line height, or use the line-height-heading?

Same thing happens with the description line height, the default 1.6 ends up making it line-height: 22.4px which is not the 20px specified above. How should we proceed?

Change 737013 had a related patch set uploaded (by Jhernandez; author: Jhernandez):

[mediawiki/extensions/QuickSurveys@master] Font size adjustments to survey designs

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

I've submitted a patch where I've explicitly set the footer and description line-heights, I can remove it if that's not what we want to do. I'm going to take screenshots and post them here.

Screenshots of the current patch:

image.png (501ร—668 px, 27 KB)

image.png (766ร—681 px, 45 KB)

image.png (510ร—673 px, 30 KB)

image.png (606ร—674 px, 51 KB)

image.png (344ร—676 px, 28 KB)

Thanks @Jhernandez. I would suggest to edit the variables in mediawiki and align those to the design style guide but I would defer to @Volker_E for approving this change (and possibly raising potential drawbacks).

Volker, while Joaquin was implementing a couple of design improvements to QuickSurvey he noticed that there are a some subtle differences between what we have defined in Figma and what is currently available in product, specifically:

  • Links color: Accent 50 #36c in Figma / #36b, #0645ad, etc. in mediawiki
  • Line height: UI Text - Regular (font size 14px) 20px or 1.4285714286 in Figma / 22.4px or 1.6 in mediawiki
  • Line height: Smaller UI Text (font size 13px) 18px or 1.3846153846 in Figma / Currently not available in mediawiki

How do you suggest to move forward? Thank you!

@aminalhazwani Besides the approach to the line heights and the link color accent, is there anything else that needs fixing?

@aminalhazwani Besides the approach to the line heights and the link color accent, is there anything else that needs fixing?

Nope, that would be it, thanks @Jhernandez! For not making this task a blocker what we could do is:

  • Implement those changes locally in QuickSurvey and add a task to the mediawiki Phab board
  • Don't implement those changes locally, use what's available in mediawiki and add a task to the QuickSurvey and/or mediawiki Phab board

Based on your experience what's the best strategy regarding design (and potentially developer) debt?

Nope, that would be it, thanks @Jhernandez!

Awesome!

For not making this task a blocker what we could do is:

  • Implement those changes locally in QuickSurvey and add a task to the mediawiki Phab board
  • Don't implement those changes locally, use what's available in mediawiki and add a task to the QuickSurvey and/or mediawiki Phab board

Based on your experience what's the best strategy regarding design (and potentially developer) debt?

For the line-height changes, I've included them in the patch directly in QuickSurveys and is what you see in the screenshots I believe. The reason is because I saw that wvui includes the style guide line-heights for its components so it doesn't feel like I'm hardcoding something that won't be used anywhere else, but it is already present in other places.

For the link color I'm more hesitant to hard-code it in QuickSurveys since it will be more noticeable with the amount of links there are in pages, so I'd rather wait for changes to upstream.

My take in this situation is then:

  • We include the line-heights in QuickSurveys since they match what other UI elements will have
  • We create a design task for updating wvui and the core mediawiki variables with the right link color and line-heights
    • We discuss in that task with Volker and others about the best way to upstream this. Maybe it is mediawiki, maybe wvui, maybe codex ๐Ÿคทโ€โ™‚๏ธ who knows. But at least it doesn't block our work and we can move forward.

@aminalhazwani Would you be up for creating the task with the design details? I can review it and add any tech stuff that's missing and then we can add people to see how we would proceed.

We include the line-heights in QuickSurveys since they match what other UI elements will have

+1!

@aminalhazwani Would you be up for creating the task with the design details? I can review it and add any tech stuff that's missing and then we can add people to see how we would proceed.

Yes, I'll take care of this and add you as a subscriber so that you can see the task in Phab.

We create a design task for updating wvui and the core mediawiki variables with the right link color and line-heights

@Jhernandez I was checking if a task was already existing for the right link color. May this T213778: Update link colors in Vector for improved UX (and consistency) be it?

While for the line-height I found this relatively old task T196322: WikimediaUI theme: Reduce `line-height` values across widgets but I could not really grasp how things move on from there. I am going to create a task for the line-height on the UI-standardization board.

@Jhernandez I was checking if a task was already existing for the right link color. May this T213778: Update link colors in Vector for improved UX (and consistency) be it?

Definitely looks like it, and Kosta commented in {T213778#7401222} about upstreaming it to all skins, so hopefully that is what will happen in that task.

While for the line-height I found this relatively old task T196322: WikimediaUI theme: Reduce `line-height` values across widgets but I could not really grasp how things move on from there. I am going to create a task for the line-height on the UI-standardization board.

Sounds good! Taking into account UI and content, it may be that we want to add new line-height variables for UI elements but also have the content line height. It feels like the 1.6 is ideal for content but less so for UI elements.

Test wiki created on Patch Demo by JHernandez (WMF) using patch(es) linked to this task:

https://patchdemo.wmflabs.org/wikis/5cda7b273e/w/

This is so cool, thank you for making this happen @Jhernandez. It's super helpful to be able to test patches live instead of static screenshots. Playing with the web inspector allowed me to find the perfect balance between the base 4 grid and the optical alignment. I would like to suggest the following changes, and then we're good to go!

.ext-quick-survey-panel .survey-header strong {
    margin: 4px 16px 0; // instead of 2px top and 0 right
    flex: 1;
}

.ext-quick-survey-panel .content > :not(p):not(:first-child) { // exclude paragraphs
  margin: 8px 0 0;
}

.ext-quick-survey-panel .content > p { // new declaration
  margin: 4px 0 0;
}

Can you explain what you are accomplishing with these? I may have to amend the commit message and the style rules are authored in less not in the css you see so without context the CSS rules it is a bit tricky to figure out the best places to put these. I'm going to have a look nonetheless.

A bit of clarification as to why I need to understand the reasoning behind this snippet of code:

.ext-quick-survey-panel .survey-header strong {
    margin: 4px 16px 0; // instead of 2px top and 0 right
    flex: 1;
}

This margin rule adds a 16px margin that pushes the start of the question 32px away from the boundary of the box. Why?

image.png (191ร—220 px, 6 KB)

.ext-quick-survey-panel .content > :not(p):not(:first-child) { // exclude paragraphs
  margin: 8px 0 0;
}

By excluding the paragraphs, you are targeting the description indirectly, which we would want to avoid. By leaving the paragraph/description unstyled it inherits the browser's margin for paragraphs, which is 0.5em top/bottom, which in a 14px font size is 7px.

I'm not sure why target explicitly :not(p) in this rule given the next rule which targets the description and overrides the margin anyways:

.ext-quick-survey-panel .content > p { // new declaration
  margin: 4px 0 0;
}

This makes the description's top margin 4px, which may look fine because it bumps against the flex container above which has the close button which increases the height of the header, but then when you have multiple lines in the question, the question and the description are smushed together with only 4px spacing between them.

image.png (273ร—661 px, 24 KB)

Is this intentional?


It kind of sounds like we want the close button out of the flow like we did when it was positioned absolute in a previous patch, but then the issue is that it can bump against the buttons if there is no description.

How is the current design different from your specs, or how have the specs changed? It would be helpful to understand how to structure markup and styles for all the different combinations.

If it is hard to explain, we can video call and talk through it, it may be easier.

This makes the description's top margin 4px, which may look fine because it bumps against the flex container above which has the close button which increases the height of the header, but then when you have multiple lines in the question, the question and the description are smushed together with only 4px spacing between them.

Is this intentional?

No, my bad. I meant to add the 16px margin on the right, not on the left ๐Ÿคฆโ€โ™‚๏ธ

I'm not sure why target explicitly :not(p) in this rule given the next rule which targets the description and overrides the margin anyways:

Good point, it def makes sense what you propose. The rule I added is unnecessary.

Just for context, I am suggested this changes becasue I noticed that Figma and the browser render the line height (and bounding box) differently, would be interesting to investigate this, I think it's related to the inherit letter height of the font (Figma on the left, web screenshot on the right)

image.png (778ร—1 px, 205 KB)

Anyway in summary, this is what I am proposing:

CurrentDesiredRedlinesNotes
image.png (563ร—600 px, 74 KB)
image.png (559ร—600 px, 74 KB)
A.
Group 1.jpg (384ร—1 px, 138 KB)
B.
image.png (330ร—640 px, 51 KB)
A. Increase the margin top of the survey question to 4px so that the first line of text is vertically centered with the "x" close button. Add a margin right of 16px to the survey question so that the question does not "touch" the "x" close button when hover/active/focus is enabled. B. Reduce the margin top of only the survey description to 4px so that we account for the "extra white space" that we get from the line height of the paragraph.

I hope this does make sense, happy to hop in a call if necessary -- and sorry for the confusion @Jhernandez ๐Ÿ™‡โ€โ™‚๏ธ

Awesome makes lots of sense @aminalhazwani, I'll get on it.

I think the bounding box in the browser includes the lineheight, which maybe figma doesn't, something that would be interesting to investigate.

Test wiki created on Patch Demo by JHernandez (WMF) using patch(es) linked to this task:

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

Looking sharp! ๐Ÿ™๐Ÿผ

Change 737013 merged by jenkins-bot:

[mediawiki/extensions/QuickSurveys@master] Font size adjustments to survey designs

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

aminalhazwani changed the subtype of this task from "Design" to "Task".Wed, Nov 17, 1:29 PM

This looks good so I'll mark it as resolved