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.
<rdar://problem/72442309>
Created attachment 417183 [details] Patch v1.0
Created attachment 417184 [details] Screenshot of Patch v1.0
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`?
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.
Created attachment 417283 [details] Patch v1.1 - Review Notes
Created attachment 417284 [details] Screenshot of Patch v1.1
ChangeLog entry in Source/WebInspectorUI/ChangeLog contains OOPS!.
Created attachment 417294 [details] Patch v1.2 - Review Notes
Committed r271329: <https://trac.webkit.org/changeset/271329> All reviewed patches have been landed. Closing bug and clearing flags on attachment 417294 [details].