@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?

In T294908#7484547, @Jhernandez wrote:

@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:

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?

In T294908#7489197, @aminalhazwani wrote:

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 2022 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.

In T294908#7491845, @aminalhazwani wrote:

@Jhernandez I was checking if a task was already existing for the right link color. May this T213778: Update link colors in Vector 2022 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/

Hey @aminalhazwani I've made a patchdemo wiki with this patch, so you can check them surveys yourself before they are merged 😄, you already approved but if you want to try it live here is the chance:

• https://patchdemo.wmflabs.org/wikis/5cda7b273e/wiki/New_York_City?quicksurvey=internal%20example%20survey
• https://patchdemo.wmflabs.org/wikis/5cda7b273e/wiki/New_York_City?quicksurvey=internal%20example%20survey%20with%20description%20and%20freeform%20text
• https://patchdemo.wmflabs.org/wikis/5cda7b273e/wiki/New_York_City?quicksurvey=internal%20multiple%20answer%20example%20survey%20with%20description%20and%20freeform%20text
• https://patchdemo.wmflabs.org/wikis/5cda7b273e/wiki/New_York_City?quicksurvey=external%20example%20survey

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:

In T294908#7498725, @aminalhazwani wrote:

.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?

.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.

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)

Anyway in summary, this is what I am proposing:

Current	Desired	Redlines	Notes
		A. B.	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/

New demo wiki:

• https://patchdemo.wmflabs.org/wikis/a4d5b62aac/wiki/New_York_City?quicksurvey=internal%20example%20survey
• https://patchdemo.wmflabs.org/wikis/a4d5b62aac/wiki/New_York_City?quicksurvey=internal%20example%20survey%20with%20description%20and%20freeform%20text
• https://patchdemo.wmflabs.org/wikis/a4d5b62aac/wiki/New_York_City?quicksurvey=internal%20multiple%20answer%20example%20survey%20with%20description%20and%20freeform%20text
• https://patchdemo.wmflabs.org/wikis/a4d5b62aac/wiki/New_York_City?quicksurvey=external%20example%20survey

cc/ @aminalhazwani

Looking sharp! 🙏🏼

Change 737013 merged by jenkins-bot:

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

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

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

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

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

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