Web-stories-wp: Changing font family should result in 400 for font weight

Created on 20 May 2020  路  20Comments  路  Source: google/web-stories-wp

Bug Description

Sometimes when changing the font family, "multiple" is shown as the font weight even if multiple font weights haven't been defined within the text.

It might be that the new font family doesn't have the font weight assigned and that causes it but this needs verifying.

Expected Behaviour

  • No input field for a single selected element should ever have a "multiple" value.

Display the closest available font weight?
TBD by UX

Note: whatever is chosen should be displayed in the design panel and also for the text itself, too -- meaning applying it to the element content.

Steps to Reproduce

  1. Add text element
  2. Assign font family Open Sans Condendsed, font weight Light
  3. Now change the font family to Playball
  4. See "multiple" as the font weight.

Screenshots

Screenshot 2020-05-20 at 17.23.11.png

Additional Context

  • Plugin Version:
  • Operating System:
  • Browser:

_Do not alter or remove anything below. The following sections will be managed by moderators only._

Acceptance Criteria

Implementation Brief

Text Design Panel Workspace P2 Pea Bug

Most helpful comment

The original issue, what this ticket is trying to solve, is the scenario in which:

  • I start with an element with font family A, and a valid font weight (let's say Bold)
  • I switch font family from A to B, and suddenly the font weight drop down shows "multiple"

We want to avoid that scenario. Since there is actually just one font weight being used, not multiple, we shouldn't show multiple because the font wasn't resetting to a valid, available font weight.

In the GIF you added https://github.com/google/web-stories-wp/pull/4734#issuecomment-704347155 I see that the base case is that you do have multiple font weights in the font element. In that case, when you change the font family if you still have multiple font weights it's perfectly fine to have "multiple" in the font weight drop down, both before and after changing the font family. Ideally, we'd want font weights to be one of the valid font weights for that font family, but I don't think that's required for this bug.

So, to clarify acceptance criteria for this bug;

  • We just want to ensure that if you only have one font weight in the entire text element, and you switch font families, then you don't end up with font weight shown as "multiple" just because the existing font weight is not a valid font weight for the new font family.
  • In that scenario, it's preferable to reset to something else -- we agreed it was simplest to reset to regular / 400 or closest available.

All 20 comments

Note: UX Needed to decide if we should assign the closest available value or the default value if the previous font-weight is missing.

cc @barklund for visibility since you implemented processing the content of the text for getting the font weight -- perhaps there was already some discussion on what should happen if the new assigned font family doesn't have the previously assigned font weight.

There's a larger underlying issue here about actually changing the inline font weights to available ones when changing font rather than "just" showing another value in dropdown. This was left as an open question in #1323.

Updated the description for clarity to specifically mention that it should be applied to the text, too.

This also begs the question: What happens if the user tries to bold a text (using keyboard shortcut or button) with a font, that doesn't support bold? Should we disable the button even or do we use faux-bold?

CC @pbakaus, @samitron7.

/cc @o-fernandez

My vote goes to allowing 'faux bold' (system level bolding) as fallback for when there's no bold variant.

Let's say I have a text in Roboto supporting many weights and I have some text in there in all of these weights:

300
400
500
600
700
800
900

I then switch to a font, only supporting 300 and 400. What happens then? Do we add 700 as a "supported" weight even though the font doesn't support it and collapse all to the nearest font weight like so?

300 => 300
400 => 400
500 => 400
600 => 700
700 => 700
800 => 700
900 => 700

Alternatively we round down, so 600 => 400.

Or we don't support bold at all - for many grotesque display fonts faux-bold won't make sense anyway, so showing a button for it but not actually visually changing the font would be senseless.

We discussed this in a recent call and agreed to fixing this. No "multiple" shown, we'll just have to do approximations in some cases. When switching fonts it is ok to approximate to closest supported weight and/or default to "normal".

@miina "Multiple" is a bug. To be clear, no input field for a single selected element should ever have a "multiple" value. Multiple only appears when you select multiple elements that share a similar attribute like color, opactiy..etc.

And ditto to approximating then default to normal as a fallback

@miina "Multiple" is a bug. To be clear, no input field for a single selected element should ever have a "multiple" value. Multiple only appears when you select multiple elements that share a similar attribute like color, opactiy..etc.

@samitron7 Yes, this is a bug issue, reported to address the problem to ensure that "multiple" wouldn't appear unexpectedly. Will update the "Expected behavior" to mention that "no input field for a single selected element should ever have a "multiple" value".

Thanks Miina. I think I also filed a separate ticket a long time ago to change "multiple" to mixed because "multiple" is too long of a word to fit inside some input field and not sure where that came from >__<.

@barklund and I have been going back and forth a little in this PR to reach a clearer understanding of what this ticket should fix.

As of this comment: https://github.com/google/web-stories-wp/pull/4734#issuecomment-704347155 we reached agreement of how this should be handled. That essentially, when the font family updates, we want the font weight to return to 400 (regular) for the entire text element. If 400 is not an available weight for that new font family we want to get as close to it as possible (so either 300 or 500 and so on).

My draft PR does this for when you have the entire text element either selected (highlighted) or the text element is in focus. This however does not make it so that when you swap font families while your cursor is in the text element but the editor state has editorState.getSelection().isCollapsed or when just part of the text is highlighted that the font weight for the entire element is reset.

I think that this bug has turned into more of a feature, where we want to actually handle the reset of the font weight separately from just setting a new font weight for the text element because we need to trick the selection into being the entire text element so that we can strip any other font weights and then reapply the other styles for the text element that aren't related to the weight so that we can have consistent functionality on this reset no matter what part of the text element is selected or focused (my understanding is that we have different functionality set up for when a cursor is active in a text element, when part of a text element is highlighted and the text element is in focus).

After some discovery, it's pretty clear that we would need to alter how the fauxSelection updates or add in another update possibly to make the faux selection when resetting the weight the entire element or pass in an additional prop through the process of setting a new weight to tell functionality that it's a reset and to do something different in (probably) components/formatters/weight, components/panels/textStyle/useRichTextFormatting, components/richText/styleManipulation and components/richText/useSelectionManipulation.

Overall, this feels like a complete edge case and could possibly benefit from more discussion to figure out the best way to handle this to avoid more time getting sunk into it.

@bmattb @o-fernandez - thoughts on moving this to the backlog for the time being?

I don't think I fully follow, let me know if this is kind of true:

  • If a user changes the font family, we'll default to regular weight / 400 or the closest available for the selected font.
  • This may not happen if the user is editing the text/have some inline selection when they change the font family

Is that roughly right? I may be misreading/missing important details here, so want to check. @barklund @BrittanyIRL

Hey @o-fernandez - thanks for summarizing, this is a sticky issue (my opinion!).

If you watch these two gifs on the comment i posted: https://github.com/google/web-stories-wp/pull/4734#issuecomment-704347155 you can see the difference.

Essentially yes, that second bullet point - but my understanding based off of talking to @barklund is that we want the same functionality regardless of where you are in the text selection which is reallyyyyy tricky to not create more bugs from in the process of overriding. But if it's ok that the font weight doesn't all go back to normal when the user swaps font families while actively in the text area then this is way easier. I think there's just not enough clarity on what the acceptance criteria for this actually is.

The original issue, what this ticket is trying to solve, is the scenario in which:

  • I start with an element with font family A, and a valid font weight (let's say Bold)
  • I switch font family from A to B, and suddenly the font weight drop down shows "multiple"

We want to avoid that scenario. Since there is actually just one font weight being used, not multiple, we shouldn't show multiple because the font wasn't resetting to a valid, available font weight.

In the GIF you added https://github.com/google/web-stories-wp/pull/4734#issuecomment-704347155 I see that the base case is that you do have multiple font weights in the font element. In that case, when you change the font family if you still have multiple font weights it's perfectly fine to have "multiple" in the font weight drop down, both before and after changing the font family. Ideally, we'd want font weights to be one of the valid font weights for that font family, but I don't think that's required for this bug.

So, to clarify acceptance criteria for this bug;

  • We just want to ensure that if you only have one font weight in the entire text element, and you switch font families, then you don't end up with font weight shown as "multiple" just because the existing font weight is not a valid font weight for the new font family.
  • In that scenario, it's preferable to reset to something else -- we agreed it was simplest to reset to regular / 400 or closest available.

thanks for clarifying the acceptance criteria here, @o-fernandez - i think i have a working solution, i'm going to reexamine it first thing Monday and then I'll update the PR and clarify here that it's working as expected.

Hey @o-fernandez -

Thanks again for the clarification. It turns out that just updating the "mixed" value for single font weights is impossible because we can have a variety of ways the text element is "selected" or in focus so it's hard to tell if we have 1 font weight or some inline weights set. Given that, I combined your focused directives with feedback from Morten and have the following:

  • When there is only 1 font weight in an entire text element and you switch font families, you will not end up with "mixed" - formerly "multiple" (placeholder was updated over the lifespan of this branch).
  • When an existing font weight is not available when you make said switch of font families, we will get the closest available weight to that previously existing weight.
  • When you have multiple font weights and you switch weights, if you have the entire text element in focus you will still see "mixed", BUT when you enter that text element, if you had "light" selected but now only regular or bold are options, it'll update the selected weight to be regular when you make that portion of text element active again.

I believe this takes care of the bug and could allow future edge cases surrounding the specifics to inline styles and the faux selection process that can sometimes inhibit the above updates occurring to be handled on a granular level with the attention it'll deserve since that will necessitate updates to more functionality.

Thoughts?

That sounds good, @BrittanyIRL, thanks much!

Verified in QA

Verified in QA

Was this page helpful?
0 / 5 - 0 ratings