Modify the styles of the existing editors to fix bugs and positioning issues.
<rdar://problem/22283851>
Created attachment 258994 [details] Patch
Comment on attachment 258994 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258994&action=review > Source/WebInspectorUI/ChangeLog:35 > + Due to a current bug, you cannot extend ES6 getters/setters. In order to work around this, a member Please link the bug. I have not heard about this, and we use it throughout the codebase. If this is a JSC bug, you should annotate relevant code with the bug URL, so at a later time we will know why it was done that way (and if the workaround can be removed). > Source/WebInspectorUI/UserInterface/Views/VisualStyleKeywordCheckbox.css:47 > + content: "Small Caps"; If we do this, then the string can't be localized. > Source/WebInspectorUI/UserInterface/Views/VisualStyleRelativeNumberSlider.js:70 > + this._origin = this._startingValue; Isn't a number line's origin always 0, in the mathematical sense? This code seems to assign this._origin to outside the range represented by this._scale. I am confused by what this code is doing.
Comment on attachment 258994 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258994&action=review >> Source/WebInspectorUI/UserInterface/Views/VisualStyleKeywordCheckbox.css:47 >> + content: "Small Caps"; > > If we do this, then the string can't be localized. That is true, but we don't localize things that match the CSS keywords. We might change that stance after feedback, but I think "Small Caps" rendered in small caps makes sense here. The same effect wouldn't work if "Small Caps" was in Chinese or Japanese.
> > Source/WebInspectorUI/ChangeLog:35 > > + Due to a current bug, you cannot extend ES6 getters/setters. In order to work around this, a member > > Please link the bug. I have not heard about this, and we use it throughout > the codebase. > > If this is a JSC bug, you should annotate relevant code with the bug URL, so > at a later time we will know why it was done that way (and if the workaround > can be removed). https://bugs.webkit.org/show_bug.cgi?id=147064 > > Source/WebInspectorUI/UserInterface/Views/VisualStyleRelativeNumberSlider.js:70 > > + this._origin = this._startingValue; > > Isn't a number line's origin always 0, in the mathematical sense? > > This code seems to assign this._origin to outside the range represented by > this._scale. I am confused by what this code is doing. The origin represents the offset of the value that isn't contained in the range. For example, if the original number is 1000, we would want the origin to be 1000 so that when the user moves the slider in either direction they are adding to 1000. Another example would be if the value is 50 and cannot go negative. In this case, the origin would be 0 as the starting value is not outside of the slider's range. I'll rename some of the variables to better explain this.
Created attachment 259015 [details] Patch
Comment on attachment 259015 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259015&action=review r=me, looking nice. I will add 147064 as a "blocker" so we get pinged when it's fixed. > Source/WebInspectorUI/UserInterface/Views/VisualStyleRelativeNumberSlider.js:65 > + let midpoint = this._scale / 2; I guess the reason I found this code this confusing is that I would expect scale to be [min, max] not a scalar. It's fine for now, I guess.
Comment on attachment 259015 [details] Patch Clearing flags on attachment: 259015 Committed r188490: <http://trac.webkit.org/changeset/188490>
All reviewed patches have been landed. Closing bug.