Bug 166933

Summary: Web Inspector: spring function editor has unusual layout, should have left-aligned labels and slider tracks
Product: WebKit Reporter: BJ Burg <bburg>
Component: Web InspectorAssignee: Devin Rousso <hi>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, hi, inspector-bugzilla-changes, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 166937    
Attachments:
Description Flags
[WIP] Patch
none
Patch
none
[Image] After Patch is applied
none
Patch
bburg: review+
Patch none

Description BJ Burg 2017-01-11 10:47:12 PST
I didn't even know we had this editor!

Here are some polish things that stuck out about the layout, which make it look like a prototype:

- The labels are center-aligned. I think they should be to the left of the component sliders (in LTR)
- The sliders have no tracks. It makes it weird, I don't know the extent of where they go.
- The sliders have no labels. You can enter a value much larger than the slider's max value (say, 1000), and it looks the same as if the slider was at 100. (Maybe it should rescale if a big value is entered manually.)
- The text fields are too big (~8 characters), especially considering the default slider range is [0,100].
- Not really about the layout, but it's often the case that the popover covers up the property declaration, so I can't see how the values are used in the syntax (spring syntax seems to differ from every other timing function).
Comment 1 Radar WebKit Bug Importer 2017-01-11 11:01:54 PST
<rdar://problem/29972516>
Comment 2 Devin Rousso 2017-01-11 12:55:42 PST
(In reply to comment #0)
> I didn't even know we had this editor!

(☞゚ヮ゚)☞

> - The sliders have no tracks. It makes it weird, I don't know the extent of
> where they go.

I think this is a regression, because the style was added in <https://webkit.org/b/148120> and I distinctly remember seeing it work.  It doesn't even show up in the styles sidebar anymore.

> - The sliders have no labels. You can enter a value much larger than the
> slider's max value (say, 1000), and it looks the same as if the slider was
> at 100. (Maybe it should rescale if a big value is entered manually.)

Would it make sense to have the slider be a relative slider, meaning one that always resets to the center and just allows for faster value changing?  Do you think we even need to keep the slider?

> - Not really about the layout, but it's often the case that the popover
> covers up the property declaration, so I can't see how the values are used
> in the syntax (spring syntax seems to differ from every other timing
> function).

I agree.  This happens a lot.  Not sure how to fix it though.
Comment 3 Devin Rousso 2017-01-11 13:17:31 PST
Created attachment 298612 [details]
[WIP] Patch
Comment 4 Devin Rousso 2017-01-11 14:35:34 PST
Created attachment 298614 [details]
Patch
Comment 5 Devin Rousso 2017-01-11 14:36:21 PST
Created attachment 298615 [details]
[Image] After Patch is applied
Comment 6 Devin Rousso 2017-01-11 15:31:16 PST
Created attachment 298620 [details]
Patch
Comment 7 BJ Burg 2017-01-11 16:24:34 PST
Comment on attachment 298620 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Views/SpringEditor.css:105
> +    width: calc(75% - 10px);

Cool. Might want to use a var() here since it corresponds to the right-margin.

> Source/WebInspectorUI/UserInterface/Views/SpringEditor.js:194
> +            WebInspector.reportInternalError("Input event fired for unrecognized element");

Nice!
Comment 8 Devin Rousso 2017-01-11 17:30:29 PST
Created attachment 298641 [details]
Patch
Comment 9 WebKit Commit Bot 2017-01-11 20:12:36 PST
Comment on attachment 298641 [details]
Patch

Clearing flags on attachment: 298641

Committed r210618: <http://trac.webkit.org/changeset/210618>
Comment 10 WebKit Commit Bot 2017-01-11 20:12:40 PST
All reviewed patches have been landed.  Closing bug.