Summary: | Web Inspector: cleanup TreeOutline class and separate styles from NavigationSidebarPanel | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Matt Baker <mattbaker> | ||||||||
Component: | Web Inspector | Assignee: | Matt Baker <mattbaker> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | bburg, commit-queue, graouts, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 153034 | ||||||||||
Attachments: |
|
Description
Matt Baker
2016-01-15 14:23:25 PST
Created attachment 269136 [details]
[Patch] Proposed Fix
I checked all the tree outline instances I could find throughout the UI to make sure nothing was broken by this patch: - Content tree - DOM tree - Timelines tree - Call stack popover trees - Object trees (Element tab -> Node) - Object trees (console) As far as I can tell everything renders identically, to the pixel. Please let me know if there is a use case that I missed. Comment on attachment 269136 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=269136&action=review Nice clean up! > Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:595 > + const maximumSidebarTreeDepth = 32; Not sidebar specific now. > Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:596 > + const baseLeftPadding = 5; // Matches the padding in NavigationSidebarPanel.css for the item class. Keep in sync. TreeOutline.css, not NavigationSidebarPanel.css. > Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:604 > + styleText += "." + WebInspector.NavigationSidebarPanel.ContentTreeOutlineElementStyleClassName + childrenSubstring + " > .item { "; Should not use WebInspector.NavigationSidebarPanel.ContentTreeOutlineElementStyleClassName, it should just be .tree-outline now. (In reply to comment #4) > Comment on attachment 269136 [details] > [Patch] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=269136&action=review > > Nice clean up! > > > Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:595 > > + const maximumSidebarTreeDepth = 32; > > Not sidebar specific now. > > > Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:596 > > + const baseLeftPadding = 5; // Matches the padding in NavigationSidebarPanel.css for the item class. Keep in sync. > > TreeOutline.css, not NavigationSidebarPanel.css. > > > Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:604 > > + styleText += "." + WebInspector.NavigationSidebarPanel.ContentTreeOutlineElementStyleClassName + childrenSubstring + " > .item { "; > > Should not use > WebInspector.NavigationSidebarPanel.ContentTreeOutlineElementStyleClassName, > it should just be .tree-outline now. Fixing this broke the indentation of object tree outlines, since the generated rules take precedence. Investigating further, the generated rules don't seem to be necessary. A simple change to TreeOutline.css accomplishes the same thing: .tree-outline .children { -webkit-padding-start: 10px; } Created attachment 269200 [details]
[Image] Broken tree element highlight
Unfortunately, -webkit-padding-start produces the correct indentation but breaks the tree element highlight.
(In reply to comment #6) > Created attachment 269200 [details] > [Image] Broken tree element highlight > > Unfortunately, -webkit-padding-start produces the correct indentation but > breaks the tree element highlight. The same problem occurs using padding-left instead of -webkit-padding-start. The generated styles applied padding to list items (.item), not the ordered list (.children). This works perfectly: .tree-outline .children > .item { padding-left: 10px; } I'm not sure why we ever generated padding rules in the first place. I know we specified a maximum depth, beyond which children all had the same indentation, but it's unclear why we would want this (except to limit the amount of generated CSS). Maybe I'm missing something? The above solution doesn't work, I was using the wrong build. A possible solution is to add a tree outline class name for tree's that use the generated indentation rules, and provide a property (something like `customIndentation`.) to control the behavior. Created attachment 269209 [details]
[Patch] Proposed Fix
Comment on attachment 269209 [details] [Patch] Proposed Fix Clearing flags on attachment: 269209 Committed r195303: <http://trac.webkit.org/changeset/195303> All reviewed patches have been landed. Closing bug. |