Fix tab sidebar layout (#5677)

chrome/content/zotero/contextPane.js

@@ -31,7 +31,6 @@ var ZoteroContextPane = new function () {
 	let _contextPaneSplitterStacked;
 	let _librarySidenav;
 	let _readerSidenav;
-	let _sidePaneState;
 
 	Object.defineProperty(this, 'activeEditor', {
 		get: () => _contextPaneInner.activeEditor
@@ -70,42 +69,6 @@ var ZoteroContextPane = new function () {
 		_loadingMessageContainer.classList.toggle('hidden', !isShow);
 	};
 
-	this.getSidePaneState = (tabType) => {
-		if (!_sidePaneState) {
-			_loadSidePaneState();
-		}
-		if (!_sidePaneState[tabType]) {
-			_sidePaneState[tabType] = {
-				width: 0,
-				open: false,
-			};
-		}
-		return _sidePaneState[tabType];
-	};
-
-	this.updateSidePaneState = (tabType, state) => {
-		if (!_sidePaneState) {
-			_loadSidePaneState();
-		}
-		if (!_sidePaneState[tabType]) {
-			_sidePaneState[tabType] = {};
-		}
-		state = state || {};
-		let hasChanges = false;
-		for (let key in state) {
-			if (_sidePaneState[tabType][key] !== state[key]) {
-				hasChanges = true;
-				break;
-			}
-		}
-		if (!hasChanges) {
-			return _sidePaneState[tabType];
-		}
-		Object.assign(_sidePaneState[tabType], state);
-		_saveSidePaneState();
-		return _sidePaneState[tabType];
-	};
-
 	this.init = function () {
 		if (!Zotero) {
 			return;
@@ -120,8 +83,6 @@ var ZoteroContextPane = new function () {
 		_librarySidenav = document.querySelector("#zotero-view-item-sidenav");
 		_readerSidenav = document.getElementById('zotero-context-pane-sidenav');
 
-		_loadSidePaneState();
-
 		// Never use default status for the reader sidenav
 		_readerSidenav.toggleDefaultStatus(false);
 		
@@ -134,8 +95,6 @@ var ZoteroContextPane = new function () {
 
 	this.destroy = function () {
 		window.removeEventListener('resize', this.update);
-
-		_saveSidePaneState();
 	};
 
 	this.updateAddToNote = () => {
@@ -150,66 +109,36 @@ var ZoteroContextPane = new function () {
 	};
 
 	/**
-	 * Update the layout of the context pane and side pane.
-	 * @param {Object} options - Options for updating the layout.
-	 * @param {number | boolean} [options.sidePaneWidth] - The width of the side pane in pixels.
-	 * If boolean, it indicates whether the side pane is open (true) or collapsed (false).
-	 * @param {number} [options.contextPaneWidth] - The width of the context pane in pixels.
-	 * @returns {Object} An object containing the updated layout state.
+	 * Update the layout of the context pane
 	 */
-	this.updateLayout = ({ sidePaneWidth, contextPaneWidth } = {}) => {
+	this.updateLayout = ({ width } = {}) => {
 		let stacked = _isStacked();
+		if (typeof width !== 'number') {
+			width = _contextPane.getAttribute("width");
+		}
+		if (width && !_contextPane.style.width) {
+			_contextPane.style.width = `${width}px`;
+		}
 		let { tabContentType: tabType } = Zotero_Tabs.parseTabType();
-		let sidePaneState;
-		if (typeof sidePaneWidth === 'number') {
-			// If sidePaneWidth is a number, update the width and open state
-			sidePaneState = this.updateSidePaneState(tabType, { width: sidePaneWidth, open: sidePaneWidth > 0 });
-		}
-		else if (typeof sidePaneWidth === 'boolean') {
-			// If sidePaneWidth is a boolean, update the open state only
-			sidePaneState = this.updateSidePaneState(tabType, { open: sidePaneWidth });
-			sidePaneWidth = sidePaneState.width || 0;
-		}
-		else {
-			// If sidePaneWidth is not provided, use the saved state
-			sidePaneState = this.getSidePaneState(tabType);
-			sidePaneWidth = sidePaneState.width || 0;
-			if (sidePaneState.open === false) {
-				sidePaneWidth = 0;
-			}
-		}
-
-		if (typeof contextPaneWidth !== 'number') {
-			contextPaneWidth = _contextPane.getAttribute("width");
-		}
-
-		let sidebarWidth = `${sidePaneWidth}px`;
-		if (contextPaneWidth && !_contextPane.style.width) {
-			_contextPane.style.width = `${contextPaneWidth}px`;
+		let sidebarState = Zotero_Tabs.getSidebarState(tabType);
+		let sidebarWidth = sidebarState?.width;
+		if (!sidebarState?.open) {
+			sidebarWidth = 0;
 		}
+		// Reserve space for sidebar
 		if (Zotero.rtl) {
 			_contextPane.style.left = 0;
-			_contextPane.style.right = stacked ? sidebarWidth : 'unset';
+			_contextPane.style.right = stacked ? `${sidebarWidth}px` : 'unset';
 		}
 		else {
-			_contextPane.style.left = stacked ? sidebarWidth : 'unset';
+			_contextPane.style.left = stacked ? `${sidebarWidth}px` : 'unset';
 			_contextPane.style.right = 0;
 		}
-
-		let placeholder = document.getElementById('zotero-reader-sidebar-pane');
-		placeholder.setAttribute('collapsed', sidebarWidth ? 'false' : 'true');
-		// Don't set width if 0 to prevent layout issues in older versions
-		if (sidePaneWidth) {
-			placeholder.setAttribute('width', sidebarWidth);
-		}
-
-		return { sidePaneState };
 	};
 
 	this.update = () => {
-		let updatedState = {};
 		if (Zotero_Tabs.selectedType === 'library') {
-			return updatedState;
+			return;
 		}
 		if (_isStacked()) {
 			_contextPaneSplitterStacked.setAttribute('hidden', false);
@@ -264,49 +193,16 @@ var ZoteroContextPane = new function () {
 			tabContent.setContextPaneOpen(!this.collapsed);
 		}
 
-		Object.assign(updatedState, this.updateLayout());
+		this.updateLayout();
 		this.updateAddToNote();
 
 		ZoteroPane.updateLayoutConstraints();
-		return updatedState;
 	};
 
 	this.togglePane = () => {
 		this.collapsed = !this.collapsed;
 	};
 
-	function _loadSidePaneState() {
-		let sidePaneState = Zotero.Prefs.get('sidePaneState') || "{}";
-		try {
-			sidePaneState = JSON.parse(sidePaneState);
-		}
-		catch {
-			sidePaneState = {};
-		}
-		_sidePaneState = sidePaneState;
-	}
-
-	function _saveSidePaneState() {
-		let sidePaneState;
-		try {
-			sidePaneState = JSON.stringify(_sidePaneState);
-		}
-		catch {
-			// Default status if serialization fails
-			sidePaneState = JSON.stringify({
-				reader: {
-					width: 0,
-					open: false,
-				},
-				note: {
-					width: 0,
-					open: false,
-				},
-			});
-		}
-		Zotero.Prefs.set('sidePaneState', sidePaneState);
-	}
-
 	function _getTabContent(tabID) {
 		if (!tabID) {
 			tabID = Zotero_Tabs.selectedID;

chrome/content/zotero/elements/contextPane.js

@@ -238,6 +238,7 @@
 			}
 
 			ZoteroContextPane.update();
+			Zotero_Tabs.updateSidebarLayout();
 		}
 
 		async _setupNotesContext(tabID) {

chrome/content/zotero/elements/tabContent.js

@@ -43,16 +43,16 @@
 		 * @returns {number | null}
 		 * @description The width of the sidebar in pixels.
 		 */
-		get sidePaneWidth() {
-			let state = ZoteroContextPane.getSidePaneState(this.tabData.type);
+		get sidebarWidth() {
+			let state = Zotero_Tabs.getSidebarState(this.tabData.type);
 			if (state) {
 				return state.width || 0;
 			}
 			return null;
 		}
 
-		set sidePaneWidth(width) {
-			ZoteroContextPane.updateLayout({ sidePaneWidth: width });
+		set sidebarWidth(width) {
+			Zotero_Tabs.updateSidebarLayout({ width });
 		}
 
 		async init() {

chrome/content/zotero/tabs.js

@@ -35,6 +35,10 @@ import TabBar from 'components/tabBar';
 const MAX_LOADED_TABS = Services.sysinfo.getProperty("memsize") / 1024 / 1024 / 1024 <= 8 ? 3 : 5;
 const UNLOAD_UNUSED_AFTER = 86400; // 24h
 
+// Keep in sync with reader/src/common/components/sidebar/sidebar-resizer.js
+const SIDEBAR_DEFAULT_WIDTH = 240; // Pixels
+const SIDEBAR_MIN_WIDTH = 180; // Pixels
+
 var Zotero_Tabs = new function () {
 	Object.defineProperty(this, 'selectedID', {
 		get: () => this._selectedID
@@ -82,6 +86,8 @@ var Zotero_Tabs = new function () {
 
 	this._hasNoteContextTypes = ['reader', 'note'];
 
+	this._sidebarState = null;
+
 	this.hasContextPane = function (type) {
 		return this._hasContextPaneTypes.includes(type);
 	};
@@ -405,6 +411,110 @@ var Zotero_Tabs = new function () {
 		}
 	};
 
+	this.updateSidebarLayout = ({ width } = {}) => {
+		let { tabContentType: tabType } = Zotero_Tabs.parseTabType();
+		let sidebarState;
+		if (typeof width === 'number') {
+			// If width is a number, update the width and open state
+			sidebarState = this.updateSidebarState(tabType, { width: width, open: width > 0 });
+		}
+		else if (typeof width === 'boolean') {
+			// If width is a boolean, update the open state only
+			sidebarState = this.updateSidebarState(tabType, { open: width });
+			width = sidebarState.width || 0;
+		}
+		else {
+			// If width is not provided, use the saved state
+			sidebarState = this.getSidebarState(tabType);
+			width = sidebarState.width || 0;
+			if (sidebarState.open === false) {
+				width = 0;
+			}
+		}
+
+		if (width) {
+			let sidebarWidth = `${width}px`;
+			let placeholder = document.getElementById('zotero-reader-sidebar-pane');
+			placeholder.setAttribute('collapsed', sidebarWidth ? 'false' : 'true');
+			placeholder.setAttribute('width', sidebarWidth);
+		}
+
+		return { sidebarState };
+	};
+
+	this.getSidebarState = (tabType) => {
+		if (!this._sidebarState) {
+			this._loadSidebarState();
+		}
+		if (!this._sidebarState[tabType]) {
+			this._sidebarState[tabType] = {
+				width: SIDEBAR_DEFAULT_WIDTH,
+				open: false,
+			};
+		}
+		return this._sidebarState[tabType];
+	};
+
+	this.updateSidebarState = (tabType, state) => {
+		if (!this._sidebarState) {
+			this._loadSidebarState();
+		}
+		if (!this._sidebarState[tabType]) {
+			this._sidebarState[tabType] = {};
+		}
+		state = state || {};
+		let hasChanges = false;
+		for (let key in state) {
+			if (this._sidebarState[tabType][key] !== state[key]) {
+				hasChanges = true;
+				break;
+			}
+		}
+		if (!hasChanges) {
+			return this._sidebarState[tabType];
+		}
+		Object.assign(this._sidebarState[tabType], state);
+		this._saveSidebarState();
+		return this._sidebarState[tabType];
+	};
+
+	this._loadSidebarState = () => {
+		let sidebarState = Zotero.Prefs.get('sidebarState') || '{}';
+		try {
+			sidebarState = JSON.parse(sidebarState);
+			for (let tabType in sidebarState) {
+				if (typeof sidebarState[tabType].width !== 'number' || sidebarState[tabType].width < SIDEBAR_MIN_WIDTH) {
+					sidebarState[tabType].width = SIDEBAR_DEFAULT_WIDTH;
+				}
+			}
+		}
+		catch {
+			sidebarState = {};
+		}
+		this._sidebarState = sidebarState;
+	};
+
+	this._saveSidebarState = () => {
+		let sidebarState;
+		try {
+			sidebarState = JSON.stringify(this._sidebarState);
+		}
+		catch {
+			// Default status if serialization fails
+			sidebarState = JSON.stringify({
+				reader: {
+					width: SIDEBAR_DEFAULT_WIDTH,
+					open: false,
+				},
+				note: {
+					width: SIDEBAR_DEFAULT_WIDTH,
+					open: false,
+				},
+			});
+		}
+		Zotero.Prefs.set('sidebarState', sidebarState);
+	};
+
 	this.getTabIDByItemID = function (itemID) {
 		let tab = this._tabs.find(tab => tab.data && tab.data.itemID === itemID);
 		return tab && tab.id;
@@ -428,6 +538,12 @@ var Zotero_Tabs = new function () {
 				onLoad={this._update.bind(this)}
 			/>
 		);
+
+		this._loadSidebarState();
+	};
+
+	this.destroy = function () {
+		this._saveSidebarState();
 	};
 
 	// When an item is modified, update the title accordingly

chrome/content/zotero/xpcom/data/notes.js

@@ -164,12 +164,12 @@ Zotero.Notes = new function () {
 		noteEditor.setBottomPlaceholderHeight(height);
 	};
 
-	this.toggleSidePane = function (_open) {
-		// TODO: Implement this once the note editor supports side pane
+	this.toggleSidebar = function (_open) {
+		// TODO: Implement this once the note editor supports side bar
 	};
 
-	this.setSidePaneWidth = function () {
-		// TODO: Implement this once the note editor supports side pane
+	this.setSidebarWidth = function () {
+		// TODO: Implement this once the note editor supports side bar
 	};
 
 	this.setContextPaneOpen = function (noteEditor, open) {
@@ -177,9 +177,11 @@ Zotero.Notes = new function () {
 	};
 
 	this._updateLayout = function () {
-		let { sidePaneState } = Zotero.getMainWindow().ZoteroContextPane.update();
-		this.toggleSidePane(sidePaneState.open);
-		this.setSidePaneWidth(sidePaneState.width);
+		let win = Zotero.getMainWindow();
+		win.ZoteroContextPane.update();
+		let { sidebarState } = win.Zotero_Tabs.updateSidebarLayout();
+		this.toggleSidebar(sidebarState.open);
+		this.setSidebarWidth(sidebarState.width);
 	};
 
 	this.getByTabID = function (tabID) {

chrome/content/zotero/xpcom/reader.js

@@ -1466,10 +1466,10 @@ class ReaderTab extends ReaderInstance {
 
 		this._onToggleSidebarCallback = (open) => {
 			if (open) {
-				this._window.ZoteroContextPane.updateLayout({ sidePaneWidth: true });
+				this._window.Zotero_Tabs.updateSidebarLayout({ width: true });
 			}
 			else {
-				this._window.ZoteroContextPane.updateLayout({ sidePaneWidth: false });
+				this._window.Zotero_Tabs.updateSidebarLayout({ width: false });
 			}
 
 			if (options.onToggleSidebar) {
@@ -1478,7 +1478,7 @@ class ReaderTab extends ReaderInstance {
 		};
 		
 		this._onChangeSidebarWidthCallback = (width) => {
-			this._window.ZoteroContextPane.updateLayout({ sidePaneWidth: width });
+			this._window.Zotero_Tabs.updateSidebarLayout({ width });
 
 			if (options.onChangeSidebarWidth) {
 				options.onChangeSidebarWidth(width);
@@ -1591,9 +1591,9 @@ class ReaderTab extends ReaderInstance {
 	}
 
 	_updateLayout() {
-		let { sidePaneState } = this._window.ZoteroContextPane.updateLayout();
-		this.toggleSidebar(sidePaneState.open);
-		this.setSidebarWidth(sidePaneState.width);
+		let { sidebarState } = this._window.Zotero_Tabs.updateSidebarLayout();
+		this.toggleSidebar(sidebarState.open);
+		this.setSidebarWidth(sidebarState.width);
 	}
 }
 
@@ -2183,7 +2183,7 @@ class Reader {
 	_loadSidebarState() {
 		let win = Zotero.getMainWindow();
 		if (win) {
-			let state = win.ZoteroContextPane.getSidePaneState('reader');
+			let state = win.Zotero_Tabs.getSidebarState('reader');
 			this._sidebarOpen = state.open;
 			if (state.width) {
 				this._sidebarWidth = parseInt(state.width);

chrome/content/zotero/zoteroPane.js

@@ -756,6 +756,7 @@ var ZoteroPane = new function () {
 		observerService.removeObserver(_reloadObserver, "zotero-reloaded");
 		
 		ZoteroContextPane.destroy();
+		Zotero_Tabs.destroy();
 
 		if (!Zotero.getZoteroPanes().length) {
 			Zotero.Session.setLastClosedZoteroPaneState(this.getState());
@@ -6738,7 +6739,8 @@ var ZoteroPane = new function () {
 			ZoteroPane.itemsView._updateHeight();
 		}
 		ZoteroContextPane.update();
-	}
+		Zotero_Tabs.updateSidebarLayout();
+	};
 	
 	
 	this.getState = function () {