Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix split view splitters being offset by sidebar #4088

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
+7 −6
Open

Fix split view splitters being offset by sidebar #4088

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aa30854
Fix split view splitters being offset by sidebar
vintydong Dec 31, 2024
e5b7c61
Move spltiview overlay inside tabpanels
vintydong Dec 31, 2024
4181185
Revert "Fix split view splitters being offset by sidebar"
vintydong Dec 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions src/browser/base/zen-components/ZenViewSplitter.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,12 @@ class ZenViewSplitter extends ZenDOMOperatedFeature {
() => document.getElementById('zen-splitview-dropzone')
);

ChromeUtils.defineLazyGetter(
this,
'sidebarBox',
() => document.getElementById('sidebar-box')
);

window.addEventListener('TabClose', this.handleTabClose.bind(this));
this.initializeContextMenu();
this.insertPageActionButton();
Expand Down Expand Up @@ -217,6 +223,31 @@ class ZenViewSplitter extends ZenDOMOperatedFeature {
wrapper.setAttribute('hidden', !value);
}

/**
* Updates the padding on the wrapper to account for sidebar width
* and properly align vertical splitters
*/
updateWrapperDisplay() {
const wrapper = this.overlay?.parentNode;
if (!wrapper) return;

const isRight = this.sidebarBox.getAttribute('positionend') !== null;
const sidebarShown = this.sidebarBox.getAttribute('hidden') === null;
const sidebarWidth = this.sidebarBox.style.width;

if (!sidebarShown) {
wrapper.style.paddingLeft = 0;
wrapper.style.paddingRight = 0;
}
else if (isRight) {
wrapper.style.paddingLeft = 0;
wrapper.style.paddingRight = sidebarWidth;
} else {
wrapper.style.paddingLeft = sidebarWidth;
wrapper.style.paddingRight = 0;
}
}

enableTabRearrangeView() {
if (this.rearrangeViewEnabled) return;
this.rearrangeViewEnabled = true;
Expand Down Expand Up @@ -750,6 +781,7 @@ class ZenViewSplitter extends ZenDOMOperatedFeature {
this.updateSplitViewButton(false);
this.applyGridToTabs(splitData.tabs);
this.applyGridLayout(splitData.layoutTree);
this.updateWrapperDisplay();
this.toggleWrapperDisplay(true);
}

Expand Down
41 changes: 37 additions & 4 deletions src/browser/components/sidebar/browser-sidebar-js.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,35 @@
diff --git a/browser/components/sidebar/browser-sidebar.js b/browser/components/sidebar/browser-sidebar.js
index 4a124003976684e014435854aef69ce29da541d2..61ce44751c36eea3e5ae2ddcc62e42c01459629b 100644
index e95aa93e912d97aca2f60bf094562ea43a8d179b..166497ed26b075995b2151c5f841dc5dd6d20699 100644
--- a/browser/components/sidebar/browser-sidebar.js
+++ b/browser/components/sidebar/browser-sidebar.js
@@ -578,7 +578,7 @@ var SidebarController = {
@@ -246,6 +246,8 @@ var SidebarController = {
_localesObserverAdded: false,
_mainResizeObserverAdded: false,
_mainResizeObserver: null,
+ _sidebarResizeObserverAdded: false,
+ _sidebarResizeObserver: null,
_ongoingAnimations: [],

/**
@@ -334,6 +336,17 @@ var SidebarController = {
sidebarBox.style.maxWidth = `calc(75vw - ${entry.contentBoxSize[0].inlineSize}px)`;
});

+ if (this._sidebarResizeObserver) {
+ this._sidebarResizeObserver.disconnect();
+ }
+ this._sidebarResizeObserver = new ResizeObserver(async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think we should add an observer, I believe there are better ways...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found an easier solution by simply moving the splitview-overlay and wrapper into the tabbrowser so the inset % will be correct based on the tabpanels width.

I think it makes more sense for splitview overlay to be nested within the tabpanel so its always the correct size vs. doing extra calculations. I also removed the xhtml file since it seemed to be the occurrence

+ gZenViewSplitter.updateWrapperDisplay();
+ })
+ if (!this._sidebarResizeObserverAdded) {
+ this._sidebarResizeObserver.observe(this._box);
+ this._sidebarResizeObserverAdded = true;
+ }
+
if (this.sidebarRevampEnabled) {
if (!customElements.get("sidebar-main")) {
ChromeUtils.importESModule(
@@ -654,7 +667,7 @@ var SidebarController = {
*/
setPosition() {
// First reset all ordinals to match DOM ordering.
Expand All @@ -11,7 +38,7 @@ index 4a124003976684e014435854aef69ce29da541d2..61ce44751c36eea3e5ae2ddcc62e42c0
[...browser.children].forEach((node, i) => {
node.style.order = i + 1;
});
@@ -592,9 +592,10 @@ var SidebarController = {
@@ -668,9 +681,10 @@ var SidebarController = {
let boxOrdinal = this._box.style.order;
this._box.style.order = tabbox.style.order;

Expand All @@ -24,7 +51,7 @@ index 4a124003976684e014435854aef69ce29da541d2..61ce44751c36eea3e5ae2ddcc62e42c0
// Indicate we've switched ordering to the box
this._box.toggleAttribute("positionend", true);
sidebarMain.toggleAttribute("positionend", true);
@@ -603,6 +604,9 @@ var SidebarController = {
@@ -681,9 +695,15 @@ var SidebarController = {
this._box.toggleAttribute("positionend", false);
sidebarMain.toggleAttribute("positionend", false);
sidebarContainer.toggleAttribute("positionend", false);
Expand All @@ -34,3 +61,9 @@ index 4a124003976684e014435854aef69ce29da541d2..61ce44751c36eea3e5ae2ddcc62e42c0
this.toolbarButton &&
this.toolbarButton.toggleAttribute("positionend", false);
}
+
+ // Update split view wrapper padding
+ gZenViewSplitter.updateWrapperDisplay();

this.hideSwitcherPanel();