Bug 166933

Summary: Web Inspector: spring function editor has unusual layout, should have left-aligned labels and slider tracks
Product: WebKit Reporter: Blaze 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

Blaze Burg
Reported 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).
Attachments
[WIP] Patch (4.90 KB, patch)
2017-01-11 13:17 PST, Devin Rousso
no flags
Patch (6.15 KB, patch)
2017-01-11 14:35 PST, Devin Rousso
no flags
[Image] After Patch is applied (64.12 KB, image/png)
2017-01-11 14:36 PST, Devin Rousso
no flags
Patch (6.39 KB, patch)
2017-01-11 15:31 PST, Devin Rousso
bburg: review+
Patch (6.62 KB, patch)
2017-01-11 17:30 PST, Devin Rousso
no flags
Radar WebKit Bug Importer
Comment 1 2017-01-11 11:01:54 PST
Devin Rousso
Comment 2 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.
Devin Rousso
Comment 3 2017-01-11 13:17:31 PST
Created attachment 298612 [details] [WIP] Patch
Devin Rousso
Comment 4 2017-01-11 14:35:34 PST
Devin Rousso
Comment 5 2017-01-11 14:36:21 PST
Created attachment 298615 [details] [Image] After Patch is applied
Devin Rousso
Comment 6 2017-01-11 15:31:16 PST
Blaze Burg
Comment 7 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!
Devin Rousso
Comment 8 2017-01-11 17:30:29 PST
WebKit Commit Bot
Comment 9 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>
WebKit Commit Bot
Comment 10 2017-01-11 20:12:40 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.