RESOLVED FIXED 160321
Web Inspector: Popover for Color Picker should update position when window resizes 
https://bugs.webkit.org/show_bug.cgi?id=160321
Summary Web Inspector: Popover for Color Picker should update position when window re...
Joseph Pecoraro
Reported 2016-07-28 15:24:56 PDT
Summary: Popover for Color Picker should update position when window resizes. Steps to Reproduce: 1. Inspect <body> on webkit.org 2. Show Elements Tab 3. Show Styles - Rules sidebar 4. Click on a Color swatch to show Color Picker Popover 5. Horizontally resize the inspector window => Popover does not update its position to track the element it is for Notes: - This is a common problem. We solved it for the LayerDetailsSidebarPanel. We should solve it here and more generally.
Attachments
[PATCH] Proposed Fix (10.78 KB, patch)
2016-07-28 15:41 PDT, Joseph Pecoraro 		no flags
DetailsFormatted DiffDiff
[PATCH] Proposed Fix (16.79 KB, patch)
2016-07-28 16:07 PDT, Joseph Pecoraro 		bburg: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2016-07-28 15:25:10 PDT
<rdar://problem/27598358>
Joseph Pecoraro
Comment 2 2016-07-28 15:41:31 PDT
Created attachment 284824 [details] [PATCH] Proposed Fix This fixes color swatch case, and moves all the older custom implementations over to the newer, simpler version.
Joseph Pecoraro
Comment 3 2016-07-28 15:53:33 PDT
Comment on attachment 284824 [details] [PATCH] Proposed Fix I'll address a few other popovers that aren't updating for window resizes.
Joseph Pecoraro
Comment 4 2016-07-28 16:07:41 PDT
Created attachment 284828 [details] [PATCH] Proposed Fix This updates popover repositioning on window resize for most popovers. It removes custom code for: - layer details sidebar - timeline datagrid - watch expressions Adds resize handling for: - color swatches - probe expressions - visual sidebar help text - heap snapshot popovers Does not change: (Right now these do not need adjustments with window resize... The top+left is enough. But They may in the future though) - debugger popovers - breakpoint popovers
Blaze Burg
Comment 5 2016-07-29 10:32:56 PDT
Comment on attachment 284828 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=284828&action=review r=me > Source/WebInspectorUI/ChangeLog:16 > + * UserInterface/Views/HeapSnapshotDataGridTree.js: Nit: pls newline > Source/WebInspectorUI/UserInterface/Views/HeapSnapshotDataGridTree.js:75 > + set popoverNode(x) { this._popoverNode = x; } Nit: popoverGridNode > Source/WebInspectorUI/UserInterface/Views/HeapSnapshotDataGridTree.js:83 > + this._popover.windowResizeHandler = () => { If the popover is not visible when the resize handler is called, will this make it pop up? > Source/WebInspectorUI/UserInterface/Views/Popover.js:152 > + window.removeEventListener("resize", this, true); I think it's worth mentioning in the change log that the resize handler is only called while the Popover is being presented. > Source/WebInspectorUI/UserInterface/Views/ProbeSetDetailsSection.js:135 > + let target = WebInspector.Rect.rectFromClientRect(event.target.element.getBoundingClientRect()); Nit: I would call this resizedTarget to emphasize we can't reuse `target` from above which is probably out of date. > Source/WebInspectorUI/UserInterface/Views/ScopeChainDetailsSidebarPanel.js:-373 > - window.addEventListener("resize", this._windowResizeListener); Nice.
Joseph Pecoraro
Comment 6 2016-08-08 12:15:20 PDT
(In reply to comment #5) > Comment on attachment 284828 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=284828&action=review > > > Source/WebInspectorUI/UserInterface/Views/HeapSnapshotDataGridTree.js:83 > > + this._popover.windowResizeHandler = () => { > > If the popover is not visible when the resize handler is called, will this > make it pop up? The resize handler can only be called when the popover is visible. Since now Popover.js handles adding/removing the listener when the popover is shown/hidden. > > Source/WebInspectorUI/UserInterface/Views/Popover.js:152 > > + window.removeEventListener("resize", this, true); > > I think it's worth mentioning in the change log that the resize handler is > only called while the Popover is being presented. Okay.
Joseph Pecoraro
Comment 7 2016-08-08 13:58:37 PDT
<https://trac.webkit.org/changeset/204264>
Note You need to log in before you can comment on or make changes to this bug.