Inspector does not respect panel layout
Categories
(DevTools :: Inspector, enhancement, P2)
Tracking
(firefox157 fixed)
| Tracking | Status | |
|---|---|---|
| firefox157 | --- | fixed |
People
(Reporter: shane, Assigned: oconnerrev)
References
(Blocks 1 open bug)
Details
Attachments
(5 files)
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:71.0) Gecko/20100101 Firefox/71.0
Steps to reproduce:
Dock the inspector to the right with html and css open. They are split vertically (html on top and css below it) - when the panel is resized at a certain width the css splits to the right of the html instead of staying below it.
Actual results:
The html and css did not stay split vertically (html on top and css below it). They instead became to very small columns side by side instead of staying in a single column split horizontally - html on top and css below it.
Expected results:
The HTML should always stay on top and the css should always stay below it.
Toggle Option would be nice to change how panels splits and resize. The vertical 2-column split makes the rules very hard to read and is disorienting when trying to look at a wide view of html and having the css "snap-in" next to it in a column, effectively making the html view smaller and impossible to read.
The image attached shows what is happening (left) and what should happen regardless of devtools width (right).
An option or toggle would be nice ("Lock devtools panel layout" or "Single column with horizontal split panels")
Comment 1•6 years ago
|
||
Bugbug thinks this bug should belong to this component, but please revert this change in case of error.
Comment 2•6 years ago
|
||
Thanks for filing. I've heard the same feedback before. It's interesting because the logic right now is such that the inspector tries to adapt to whatever real estate is available and make the most of it. That's why it decide on its own to either stack things vertically or horizontally. Related to this: in 3-panel mode, we're making changes right now so it would switch from 3 rows (if very narrow), to 1 row on top and 2 columns below (if somewhat narrow), all the way to 3 columns (if very wide). But what you're saying is, on contrary, that you do not necessarily want things to change on their own while you are resizing.
And I have to admit I've myself also been frustrated by this when I'm used to a certain layout (say everything in columns) and I just want to make devtools bigger for some time but keep that layout. Well after some point things jump to another layout and I have to get used to the way things look now.
So an option or toggle might indeed make sense.
Martin: maybe some new food for thoughts to consider while we're working on this 3-panel layout.
Comment 3•6 years ago
|
||
That's a good idea. We've struggled quite a bit to find a good solution, so giving the user the option to control the layout sounds like a good idea to me 👍
Updated•6 years ago
|
Updated•3 years ago
|
Comment 6•10 months ago
|
||
me bumped into this as i switched to vertical monitor.
Comment 7•10 months ago
|
||
i found a temporary solution for now, if i zoom in a few times, the panel moves to the bottom
| Assignee | ||
Comment 8•2 months ago
|
||
Hey so this has been bugging me forever, I dove into browser code because of this.
this is the solution:
I am not sure if its okay to put the edited files here so I am writing the snippets needed to make it happen.
It's my first time here so apologies if I am not supposed to do this.
devtools\client\locales\en-US\inspector.properties
# LOCALIZATION NOTE (inspector.splitOrientation.tooltip): Tooltip for the
button that toggles the inspector splitter between landscape and portrait.
inspector.splitOrientation.tooltip=Toggle split orientation
devtools\client\inspector\inspector.js
const THREE_PANE_ENABLED_PREF = "devtools.inspector.three-pane-enabled";
const THREE_PANE_CHROME_ENABLED_PREF =
"devtools.inspector.chrome.three-pane-enabled";
// Forces the splitter orientation: "landscape", "portrait", or "auto" to
// keep the width-threshold behavior.
const SPLIT_ORIENTATION_PREF = "devtools.inspector.split-orientation";
const DEFAULT_COLOR_UNIT_PREF = "devtools.defaultColorUnit";
/**
* Check if the inspector should use the landscape mode.
*
* @return {boolean} true if the inspector should be in landscape mode.
*/
#useLandscapeMode() {
if (!this.panelDoc) {
return true;
}
const orientation = Services.prefs.getCharPref(
SPLIT_ORIENTATION_PREF,
"auto"
);
if (orientation === "landscape" || orientation === "portrait") {
return orientation === "landscape";
}
const splitterBox = this.panelDoc.getElementById("inspector-splitter-box");
const width = splitterBox.clientWidth;
#onLazyPanelResize = debounce(
() => {
// We can be called on a closed window or destroyed toolbox because of the deferred task.
if (
this.panelWin?.closed ||
this.#destroyed ||
this.#toolbox.currentToolId !== "inspector"
) {
return;
}
this.splitBox.setState({ vert: this.#useLandscapeMode() });
this.emit("inspector-resize");
},
LAZY_RESIZE_INTERVAL_MS,
this
);
onSplitOrientationButtonClicked = () => {
const useLandscape = !this.#useLandscapeMode();
Services.prefs.setCharPref(
SPLIT_ORIENTATION_PREF,
useLandscape ? "landscape" : "portrait"
);
this.splitBox.setState({ vert: useLandscape });
this.splitOrientationButton?.setAttribute("aria-pressed", useLandscape);
};
async #setupToolbar() {
this.#teardownToolbar();
// Setup the add-node button.
this.addNodeButton = this.panelDoc.getElementById(
"inspector-element-add-button"
);
this.addNodeButton.addEventListener("click", this.addNode);
// Setup the split orientation toggle button.
this.splitOrientationButton = this.panelDoc.getElementById(
"inspector-split-orientation-toggle"
);
this.splitOrientationButton.setAttribute(
"aria-pressed",
this.#useLandscapeMode()
);
this.splitOrientationButton.addEventListener(
"click",
this.onSplitOrientationButtonClicked
);
#teardownToolbar() {
if (this.addNodeButton) {
this.addNodeButton.removeEventListener("click", this.addNode);
this.addNodeButton = null;
}
if (this.eyeDropperButton) {
this.eyeDropperButton.removeEventListener(
"click",
this.onEyeDropperButtonClicked
);
this.eyeDropperButton = null;
}
if (this.splitOrientationButton) {
this.splitOrientationButton.removeEventListener(
"click",
this.onSplitOrientationButtonClicked
);
this.splitOrientationButton = null;
}
}
\devtools\client\inspector\index.xhtml
<button
id="inspector-eyedropper-toggle"
class="devtools-button"
></button>
<button
id="inspector-split-orientation-toggle"
class="devtools-button"
aria-pressed="false"
data-localization="title=inspector.splitOrientation.tooltip"
></button>
</div>
devtools\client\themes\inspector.css
/* Split orientation toolbar button */
#inspector-split-orientation-toggle::before {
background-image: url(images/dock-bottom.svg);
background-position: center;
background-size: 14px;
}
#inspector-split-orientation-toggle[aria-pressed="true"]::before {
background-image: url(images/dock-side-right.svg);
}
browser\app\profile\firefox.js
// Enable the 3 pane mode in the inspector
pref("devtools.inspector.three-pane-enabled", true);
// Enable the 3 pane mode in the chrome inspector
pref("devtools.inspector.chrome.three-pane-enabled", false);
// Splitter orientation: "landscape", "portrait", or "auto" (width-based)
pref("devtools.inspector.split-orientation", "auto");
// Collapse pseudo-elements by default in the rule-view
pref("devtools.inspector.show_pseudo_elements", false);
| Assignee | ||
Comment 9•2 months ago
|
||
Comment 10•2 months ago
|
||
(In reply to Rév O'Conner from comment #8)
Hey so this has been bugging me forever, I dove into browser code because of this.
this is the solution:
I am not sure if its okay to put the edited files here so I am writing the snippets needed to make it happen.
It's my first time here so apologies if I am not supposed to do this.
Hello Rév, thanks a lot for the patch!
Can you submit it following https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html#to-submit-a-patch so we can start the review process ?
Let me know if you run into any issue
| Assignee | ||
Comment 11•2 months ago
|
||
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #10)
(In reply to Rév O'Conner from comment #8)
Hey so this has been bugging me forever, I dove into browser code because of this.
this is the solution:
I am not sure if its okay to put the edited files here so I am writing the snippets needed to make it happen.
It's my first time here so apologies if I am not supposed to do this.Hello Rév, thanks a lot for the patch!
Can you submit it following https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html#to-submit-a-patch so we can start the review process ?
Let me know if you run into any issue
Thanks for responding. I am a bit busy this month, so I expect to hop on it maybe by second week of August. I checked out the process, and seemed straightforward
Comment 12•1 month ago
|
||
(In reply to Rév O'Conner from comment #11)
Thanks for responding. I am a bit busy this month, so I expect to hop on it maybe by second week of August. I checked out the process, and seemed straightforward
Hello Rév, I wanted to check if that's something you're still interested in?
| Assignee | ||
Comment 13•1 month ago
|
||
Hey! thanks for checking in. I will start on this from Monday, just been busy job hunting. A man has to eat and what not!
Comment 14•1 month ago
|
||
(In reply to Rév O'Conner from comment #13)
Hey! thanks for checking in. I will start on this from Monday, just been busy job hunting. A man has to eat and what not!
sure, no worries, I don't want this to become a burden, do it if you want and have the time to, otherwise I'll take over (but I'd likely take inspiration from the investigation you did here, so I want to make sure you have a chance to get proper credit for it by doing the commit yourself :) )
| Assignee | ||
Comment 15•1 month ago
|
||
Adds a meatball-style menu to the Inspector toolbar with three radio
options: Automatic (the existing width-based behavior, default), Side by
side, and Stacked. The choice is stored in the new
devtools.inspector.split-orientation pref. A pref observer keeps the
layout and the menu in sync when the pref changes externally.
Also adds the menuitemradio role to the shared menu button styling in
tooltips.css, since MenuItem supports it but the stylesheet only covered
menuitem and menuitemcheckbox.
Updated•1 month ago
|
| Assignee | ||
Comment 16•1 month ago
|
||
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #14)
(In reply to Rév O'Conner from comment #13)
Hey! thanks for checking in. I will start on this from Monday, just been busy job hunting. A man has to eat and what not!
sure, no worries, I don't want this to become a burden, do it if you want and have the time to, otherwise I'll take over (but I'd likely take inspiration from the investigation you did here, so I want to make sure you have a chance to get proper credit for it by doing the commit yourself :) )
Hey, so I had some time on my hand and submitted the fix for this. I had to take some help from AI since I have never worked with phab before, and just overall making sure I was sticking to the process. So if there something I missed or did wrong please let me know. The AI also said to comment on the phabricator page for the bug but I cannot find any comment box or anything over here. Hah! So I am assuming it's just hallucinated that part.
I changed a few things from the code written above, since that was an adaptation from a userchrome.js script I run on my daily use firefox. I realised it would be confusing in terms of UX for anyone else. I adapted the three dot meatball menu on top "Customize developer tools and get help" with an auto which is the current auto layout, and side by side and stacked as menuitems. The styling was done with the help of browser toolbox, hopefully I have no added any duplicate CSS rules here.
I am attaching a screen recording of it in play here https://drive.google.com/file/d/1HVvoad7tCeI9GXgo4WIXzeDigNs_piLu/view?usp=sharing (no idea how to natively attach video here, so bear with me )
The test passed locally for me as well as linting and I ran tests for all areas of the browser touched.
Tests passed:
- the (NEW, included) browser_inspector_split-orientation.js
- browser_toolbox_meatball.js,
- eslint, stylelint and fluent-lint.
| Assignee | ||
Comment 17•28 days ago
|
||
@nchevobbe submitted the revision. Do i need to comment here or how does the bugzilla and phabricator link up. A bit confused still about the platforms, please excuse me.
Comment 18•23 days ago
|
||
(In reply to Rév O'Conner from comment #17)
@nchevobbe submitted the revision. Do i need to comment here or how does the bugzilla and phabricator link up. A bit confused still about the platforms, please excuse me.
hello Rev, thanks for the update. I'm just back from PTO, so I'll have a look shortly (probably tomorrow)
Comment 19•22 days ago
|
||
Comment 20•21 days ago
|
||
| bugherder | ||
Description
•