Bug 219996 – Web Inspector: Font Details sidebar - Improve visibility of values by emphasizing them/de-emphasizing range information

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Patrick Angle]

Created attachment 416472 [details]
Screenshot of Issue

In the Font details sidebar values, particularly for variation axes, are difficult to read quickly. We should either emphasize the actual value, de-emphasize the range/default information, or both.

[Radar WebKit Bug Importer]

<rdar://problem/72442309>

[Patrick Angle]

Created attachment 417183 [details]
Patch v1.0

[Patrick Angle]

Created attachment 417184 [details]
Screenshot of Patch v1.0

[Devin Rousso]

Comment on attachment 417183 [details]
Patch v1.0

View in context: https://bugs.webkit.org/attachment.cgi?id=417183&action=review

r=me, with a few fixes/questions :)

> Source/WebInspectorUI/UserInterface/Views/FontDetailsPanel.css:26
> +.font > .details-section > .content > .group > .row.simple > .value .secondary {

Starting with `.font` is super generic.  Please change it to `.sidebar > .panel.details.font` to be more specific (and match other details sidebar panels).

> Source/WebInspectorUI/UserInterface/Views/FontDetailsPanel.css:28
> +    word-break: normal;

Is this necessary?  IIRC the default value of `word-break` is `normal`.

> Source/WebInspectorUI/UserInterface/Views/FontDetailsPanel.js:180
> +        let secondaryElement = document.createElement("span");

NIT: you can combine this and 183 to be one line
```
    let secondaryElement = valueElement.appendChild(document.createElement("span"));
```

> Source/WebInspectorUI/UserInterface/Views/FontDetailsPanel.js:182
> +        secondaryElement.textContent = WI.UIString(" (Range: %d-%d)", " (Range: %d-%d) @ Font Details Sidebar", "A range for a single variation axis of a font.").format(minimumValue, maximumValue);

Do we no longer care about the `defaultValue`?

[Patrick Angle]

Comment on attachment 417183 [details]
Patch v1.0

View in context: https://bugs.webkit.org/attachment.cgi?id=417183&action=review

>> Source/WebInspectorUI/UserInterface/Views/FontDetailsPanel.css:28
>> +    word-break: normal;
> 
> Is this necessary?  IIRC the default value of `word-break` is `normal`.

`.details-section > .content > .group > .row.simple > .value` defines `word-break:break-all`, which looks odd here since this isn't just a number or code.

>> Source/WebInspectorUI/UserInterface/Views/FontDetailsPanel.js:182
>> +        secondaryElement.textContent = WI.UIString(" (Range: %d-%d)", " (Range: %d-%d) @ Font Details Sidebar", "A range for a single variation axis of a font.").format(minimumValue, maximumValue);
> 
> Do we no longer care about the `defaultValue`?

For weight/stretch the default won't be the default that is used anyways since CSS defines its own defaults there (I haven't noticed any fonts that define odd defaults for these yet, either), and for other variation axes the default will be shown as the value unless the developer has defined their own value for the axis, so this information becomes more or less redundant.

[Patrick Angle]

Created attachment 417283 [details]
Patch v1.1 - Review Notes

[Patrick Angle]

Created attachment 417284 [details]
Screenshot of Patch v1.1

[EWS]

ChangeLog entry in Source/WebInspectorUI/ChangeLog contains OOPS!.

[Patrick Angle]

Created attachment 417294 [details]
Patch v1.2 - Review Notes

[EWS]

Committed r271329: <https://trac.webkit.org/changeset/271329>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 417294 [details].