[webkit-reviews] review requested: [Bug 178404] Web Inspector: [PARITY] Styles Redesign: Add color gradient, bezier curve, and spring inline widgets : [Attachment 324430] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Oct 20 12:43:09 PDT 2017
Nikita Vasilyev <nvasilyev at apple.com> has asked for review:
Bug 178404: Web Inspector: [PARITY] Styles Redesign: Add color gradient, bezier
curve, and spring inline widgets
https://bugs.webkit.org/show_bug.cgi?id=178404
Attachment 324430: Patch
https://bugs.webkit.org/attachment.cgi?id=324430&action=review
--- Comment #8 from Nikita Vasilyev <nvasilyev at apple.com> ---
Created attachment 324430
--> https://bugs.webkit.org/attachment.cgi?id=324430&action=review
Patch
(In reply to Matt Baker from comment #4)
> Comment on attachment 324071 [details]
> Patch
>
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=324071&action=review
>
> > Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:295
> > + let value = event.data && event.data.value &&
event.data.value.toString();
>
> InlineSwatch always includes event data when the value changes, so that
> check can be removed. I think it's also safe to assume that event.data
> implies event.data.toString(). With the exception of
> Color.prototype.toString (which can throw), all the InlineSwatch value
> objects (Color, CubicBezier, Spring, and the Gradient subclasses) will
> always return a string.
>
> It's not as concise, but you could rewrite this as:
>
> if (!event.data.value)
> return;
>
> try {
> let value = event.data.value.toString();
> console.assert(value, "Expected value string.", event.data.value);
> if (!value)
> return;
> this._handleValueChange();
> } catch (e) {
> console.error("Could not convert value to string.", e);
> }
The old styled sidebar didn't have try/catch, see
CSSStyleDeclarationTextEditor.propotype._inlineSwatchValueChanged:
let value = event.data && event.data.value && event.data.value.toString();
We rarely use try/catch and it seems unnecessary here. I think it's better to
replace it with console.error.
More information about the webkit-reviews
mailing list