[webkit-reviews] review requested: [Bug 233054] Web Inspector: Add a swatch for align-items and align-self : [Attachment 446185] Patch

[bugzilla-daemon at webkit.org]

Patrick Angle <pangle at apple.com> has asked  for review:
Bug 233054: Web Inspector: Add a swatch for align-items and align-self
https://bugs.webkit.org/show_bug.cgi?id=233054

Attachment 446185: Patch

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




--- Comment #18 from Patrick Angle <pangle at apple.com> ---
Comment on attachment 446185
  --> https://bugs.webkit.org/attachment.cgi?id=446185
Patch

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

Removing r+ for now; Devin makes a good argument for a WI.AlignmentData or
similar object that will want to be reviewed.

>> Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:855
>> +	    let valueObject = {
> 
> This is really hacky :/
> 
> Can we maybe make a `WI.AlignmentData` wrapper model object so it's more
clear?	Ideally it'd also have a `WI.AlignmentData.Type` that would map/replace
the `propertyName` so that we're using a frontend value (which means it's more
easy to change) instead of one from the page/backend (even if it is a CSS
property name).
> 
> NIT: I'd just inline this

+1 good idea