Created attachment 267514 [details] [Screenshot] Hidden completion suggestions * SUMMARY Popover covers completion suggestions. This occurs because --z-index-popover is higher than --z-index-tooltip, which is used by the popover's completion suggestion view. Regressed in http://trac.webkit.org/changeset/193913.
Note that the fix should bump the z-index only for CompletionSuggestionsViews that are created from a popover.
Created attachment 267519 [details] [Patch] Proposed Fix
Comment on attachment 267519 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=267519&action=review > Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorCompletionController.js:50 > + this._suggestionsView.element.classList.add("popover"); I suspect this can be inferred from element hierarchy in the CSS selector. .popover .completion-suggestions
Comment on attachment 267519 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=267519&action=review >> Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorCompletionController.js:50 >> + this._suggestionsView.element.classList.add("popover"); > > I suspect this can be inferred from element hierarchy in the CSS selector. > > .popover .completion-suggestions Also .popover also likely conflicts and inherits styles form the real popover class.
Would anything break if we simply swap --z-index-tooltip and --z-index-popover, e.g.: --z-index-popover: 512; --z-index-tooltip: 1024; ?
Created attachment 267521 [details] [Patch] Proposed Fix
Comment on attachment 267521 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=267521&action=review > Source/WebInspectorUI/UserInterface/Views/CompletionSuggestionsView.css:33 > + z-index: calc(var(--z-index-popover) + var(--z-index-tooltip)); This is the only place where --z-index-tooltip is used. It would make more sense to simply swap --z-index-popover and --z-index-tooltip in Variables.css.
(In reply to comment #7) > Comment on attachment 267521 [details] > [Patch] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=267521&action=review > > > Source/WebInspectorUI/UserInterface/Views/CompletionSuggestionsView.css:33 > > + z-index: calc(var(--z-index-popover) + var(--z-index-tooltip)); > > This is the only place where --z-index-tooltip is used. It would make more > sense to simply swap --z-index-popover and --z-index-tooltip in > Variables.css. Nice and simple! A reordering of the z-index variables sounds like the best solution. Since only one tooltip is visible at a time, and popups are modal views that can have tooltips, I suggest the following order: --z-index-glass-pane-for-drag: 512; --z-index-uncaught-exception-sheet: 1024; --z-index-popover: 2048; --z-index-tooltip: 4096; As an aside, is there a reason why --z-index-uncaught-exception-sheet has a higher priority than --z-index-glass-pane-for-drag?
(In reply to comment #8) > (In reply to comment #7) > > Comment on attachment 267521 [details] > > [Patch] Proposed Fix > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=267521&action=review > > > > > Source/WebInspectorUI/UserInterface/Views/CompletionSuggestionsView.css:33 > > > + z-index: calc(var(--z-index-popover) + var(--z-index-tooltip)); > > > > This is the only place where --z-index-tooltip is used. It would make more > > sense to simply swap --z-index-popover and --z-index-tooltip in > > Variables.css. > > Nice and simple! A reordering of the z-index variables sounds like the best > solution. Since only one tooltip is visible at a time, and popups are modal > views that can have tooltips, I suggest the following order: > > --z-index-glass-pane-for-drag: 512; > --z-index-uncaught-exception-sheet: 1024; > --z-index-popover: 2048; > --z-index-tooltip: 4096; > > As an aside, is there a reason why --z-index-uncaught-exception-sheet has a > higher priority than --z-index-glass-pane-for-drag? --z-index-uncaught-exception-sheet is for the error page Brian recently added. It should have the highest priority as it can show up on an exception at any state. --z-index-glass-pane-for-drag is for an invisible element that covers the whole inspector. It gets added to the DOM when dragging (e.g. sidebar resizing) starts. The whole point of it is to silence all other mouse events that could have fired on other elements while resizing. It should be on top of --z-index-popover and --z-index-tooltip. I suggest to only swap --z-index-popover and --z-index-tooltip, and keep the rest untouched.
(In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #7) > > > Comment on attachment 267521 [details] > > > [Patch] Proposed Fix > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=267521&action=review > > > > > > > Source/WebInspectorUI/UserInterface/Views/CompletionSuggestionsView.css:33 > > > > + z-index: calc(var(--z-index-popover) + var(--z-index-tooltip)); > > > > > > This is the only place where --z-index-tooltip is used. It would make more > > > sense to simply swap --z-index-popover and --z-index-tooltip in > > > Variables.css. > > > > Nice and simple! A reordering of the z-index variables sounds like the best > > solution. Since only one tooltip is visible at a time, and popups are modal > > views that can have tooltips, I suggest the following order: > > > > --z-index-glass-pane-for-drag: 512; > > --z-index-uncaught-exception-sheet: 1024; > > --z-index-popover: 2048; > > --z-index-tooltip: 4096; > > > > As an aside, is there a reason why --z-index-uncaught-exception-sheet has a > > higher priority than --z-index-glass-pane-for-drag? > > --z-index-uncaught-exception-sheet is for the error page Brian recently > added. It should have the highest priority as it can show up on an exception > at any state. > > --z-index-glass-pane-for-drag is for an invisible element that covers the > whole inspector. It gets added to the DOM when dragging (e.g. sidebar > resizing) starts. The whole point of it is to silence all other mouse events > that could have fired on other elements while resizing. It should be on top > of --z-index-popover and --z-index-tooltip. > > I suggest to only swap --z-index-popover and --z-index-tooltip, and keep the > rest untouched. Sounds good!
Created attachment 267539 [details] [Patch] Proposed Fix
(In reply to comment #11) > Created attachment 267539 [details] > [Patch] Proposed Fix Looks good to me, but I'm not a reviewer.
Comment on attachment 267539 [details] [Patch] Proposed Fix Clearing flags on attachment: 267539 Committed r194214: <http://trac.webkit.org/changeset/194214>
All reviewed patches have been landed. Closing bug.