[webkit-reviews] review granted: [Bug 231432] Web Inspector: Move CSS longhand and shorthand mapping away from WI.CSSCompletions : [Attachment 440624] Patch v1.0

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Oct 8 11:36:33 PDT 2021


Devin Rousso <drousso at apple.com> has granted Razvan Caliman
<rcaliman at apple.com>'s request for review:
Bug 231432: Web Inspector: Move CSS longhand and shorthand mapping away from
WI.CSSCompletions
https://bugs.webkit.org/show_bug.cgi?id=231432

Attachment 440624: Patch v1.0

https://bugs.webkit.org/attachment.cgi?id=440624&action=review




--- Comment #3 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 440624
  --> https://bugs.webkit.org/attachment.cgi?id=440624
Patch v1.0

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

r=me

> Source/WebInspectorUI/UserInterface/Models/CSSKeywordCompletions.js:233
> +		   let shorthands =
WI.CSSKeywordCompletions.ShorthandNamesForLongHandProperty.getOrInitialize(long
hand, []);
> +		   shorthands.push(property.name);

NIT: you could instead use a `Multimap` to avoid the `getOrInitialize` (tho
you'd need to either adjust the fallback locations to be a `new Set` or change
`Multimap.prototype.get` to have a fallback)

> Source/WebInspectorUI/UserInterface/Models/CSSKeywordCompletions.js:262
> +WI.CSSKeywordCompletions.ShorthandNamesForLongHandProperty = new Map;

yknow, technically, it's a bit odd for info about properties to be on something
related to value keywords

IMO it would be better to move these to `WI.CSSCompletions` (or create a new
`WI.CSSPropertyCompletions`)


More information about the webkit-reviews mailing list