Grid / Flex highlighter icons in Rule View don't work with 3 Pane mode

RESOLVED FIXED in Firefox 59

Status

enhancement
P2
normal
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: jryans, Assigned: jryans)

Tracking

(Blocks 1 bug)

Trunk
Firefox 59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed)

Details

(Whiteboard: [designer-tools])

Attachments

(1 attachment)

If you flip the prefs to enable 3 pane inspector (bug 1369945), then the Rule View icons to toggle grid / flex highlighters (the ones that appear next `display: grid` etc.) stop working.

When 3 pane is enabled, clicking the icons enters the property editor for the `grid` value, instead of toggling highlighters.

I am guessing there's some event handler that's not wired up quite right in 3 pane mode...
Oh yes, I know this one. I think I forgot to add one of changes to the bug 1369945 patch.

We need to change the way we handle the click events in the sidebar when checking if it is the rule view. 
If we are in the split rule view, we need to do target.closest("<rule view container selector>") to see if we are in the rule view.

https://searchfox.org/mozilla-central/source/devtools/client/inspector/shared/highlighters-overlay.js#76
Whiteboard: [designer-tools]
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Priority: -- → P2
Comment on attachment 8943833 [details]
Bug 1431236 - Update grid / flex toggles for 3 pane.

https://reviewboard.mozilla.org/r/214212/#review219918

Good catch on the getNodeInfo

::: devtools/client/inspector/shared/highlighters-overlay.js:75
(Diff revision 1)
>      this.inspector.target.on("will-navigate", this.onWillNavigate);
>  
>      EventEmitter.decorate(this);
>    }
>  
> -  get isRuleView() {
> +  isRuleView(node) {

Add a JSDoc for this.

::: devtools/client/inspector/shared/highlighters-overlay.js:799
(Diff revision 1)
>      // Only one highlighter can be displayed at a time, hide the currently shown.
>      this._hideHoveredHighlighter();
>  
>      this._lastHovered = event.target;
>  
> -    let view = this.isRuleView ?
> +    let view = this.isRuleView(event.target) ?

s/event.target/this._lastHovered
Attachment #8943833 - Flags: review?(gl) → review+
Comment on attachment 8943833 [details]
Bug 1431236 - Update grid / flex toggles for 3 pane.

https://reviewboard.mozilla.org/r/214212/#review219918

> Add a JSDoc for this.

Okay, added.

> s/event.target/this._lastHovered

Changed.
Pushed by jryans@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0f66c51df9f4
Update grid / flex toggles for 3 pane. r=gl
https://hg.mozilla.org/mozilla-central/rev/0f66c51df9f4
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Blocks: 1433716
No longer blocks: 1369945
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.