Skip to content

feat: add configurable sidebar width in settings #1975

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

Open
soumik183 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add configurable sidebar width in settings #1975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 19 additions & 8 deletions src/components/sidebar/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import "./style.scss";
import toast from "components/toast";
import Ref from "html-tag-js/ref";
import actionStack from "lib/actionStack";
import appSettings from "lib/settings";
import auth, { loginEvents } from "lib/auth";
import constants from "lib/constants";

Expand Down Expand Up @@ -33,14 +34,14 @@ function create($container, $toggler) {

const START_THRESHOLD = constants.SIDEBAR_SLIDE_START_THRESHOLD_PX; //Point where to start swipe
const MIN_WIDTH = 200; //Min width of the side bar
const MAX_WIDTH = () => innerWidth * 0.7; //Max width of the side bar
const MAX_WIDTH = () => innerWidth - 40; //Max width of the side bar
const resizeBar = Ref();
const userAvatar = Ref();
const userContextMenu = Ref();

$container = $container || app;
let mode = innerWidth > 600 ? "tab" : "phone";
let width = +(localStorage.sideBarWidth || MIN_WIDTH);
let width = Math.min(appSettings.value.sidebarWidth || MIN_WIDTH, MAX_WIDTH());

const eventOptions = { passive: false };
const $el = (
Expand Down Expand Up @@ -92,6 +93,11 @@ function create($container, $toggler) {
$toggler?.addEventListener("click", toggle);
$container.addEventListener("touchstart", ontouchstart, eventOptions);
window.addEventListener("resize", onWindowResize);

appSettings.on("update:sidebarWidth", () => {
width = Math.min(appSettings.value.sidebarWidth || MIN_WIDTH, MAX_WIDTH());
setWidth(width);
});
Comment on lines +97 to +100
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 23, 2026

Choose a reason for hiding this comment

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

P1 Memory leak – listener never removed

appSettings.on("update:sidebarWidth", ...) is registered inside create() but there is no corresponding appSettings.off(...) call when the sidebar element is destroyed (e.g. after hideMaster removes it from the DOM, or if create is ever called more than once in the singleton pattern).

Each call to create() accumulates another live listener on the global appSettings object. Over time those stale callbacks will call setWidth on detached elements and keep closed-over DOM nodes alive. Store the callback reference and deregister it in a cleanup path (the existing hide/hideMaster flow would be a good place):

const onSidebarWidthUpdate = () => {
    width = Math.min(appSettings.value.sidebarWidth || MIN_WIDTH, MAX_WIDTH());
    setWidth(width);
};
appSettings.on("update:sidebarWidth", onSidebarWidthUpdate);
// … later, when the sidebar is destroyed:
// appSettings.off("update:sidebarWidth", onSidebarWidthUpdate);


if (mode === "tab" && localStorage.sidebarShown === "1") {
show();
Expand Down Expand Up @@ -226,6 +232,8 @@ function create($container, $toggler) {
localStorage.sidebarShown = 1;
$el.activated = true;
$el.onclick = null;

setWidth(width);

if (mode === "phone") {
resizeBar.style.display = "none";
Expand All @@ -239,7 +247,6 @@ function create($container, $toggler) {
action: hideMaster,
});
} else {
setWidth(width);
resizeBar.style.display = "block";
app.append($el);
$el.onclick = () => {
Expand Down Expand Up @@ -349,7 +356,8 @@ function create($container, $toggler) {
if (newWidth <= MIN_WIDTH) width = MIN_WIDTH;
else if (newWidth >= MAX_WIDTH()) width = MAX_WIDTH();
else width = newWidth;
localStorage.sideBarWidth = width;
appSettings.value.sidebarWidth = width;
appSettings.update(false);
Comment on lines +359 to +360
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 23, 2026

Choose a reason for hiding this comment

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

P2 Direct mutation triggers a redundant setWidth call

Setting appSettings.value.sidebarWidth = width directly mutates the settings object, and then appSettings.update(false) detects it as a change through #getChangedKeys(). This causes the update:sidebarWidth listener (registered at line 97) to fire, which calls setWidth(width) a second time with the exact value that was just applied.

Use the idiomatic update API instead to avoid the extra round-trip and make the intent clear:

Suggested change
appSettings.value.sidebarWidth = width;
appSettings.update(false);
appSettings.update({ sidebarWidth: width }, false);

document.removeEventListener("touchmove", onMove, eventOptions);
document.removeEventListener("mousemove", onMove, eventOptions);
document.removeEventListener("touchend", onEnd, eventOptions);
Expand Down Expand Up @@ -468,9 +476,12 @@ function create($container, $toggler) {
*/
function setWidth(width) {
$el.style.transition = "none";
$el.style.width = width + "px";
$el.style.maxWidth = width + "px";
root.style.marginLeft = width + "px";
root.style.width = `calc(100% - ${width}px)`;
if (mode === "tab") {
root.style.marginLeft = width + "px";
root.style.width = `calc(100% - ${width}px)`;
}
clearTimeout(setWidthTimeout);
setWidthTimeout = setTimeout(() => {
editorManager?.editor?.resize(true);
Expand All @@ -492,8 +503,8 @@ function create($container, $toggler) {
$el.toggle = toggle;
$el.onshow = () => {};
$el.getWidth = function () {
const width = innerWidth * 0.7;
return mode === "phone" ? (width >= 350 ? 350 : width) : MIN_WIDTH;
const configuredWidth = appSettings.value.sidebarWidth || MIN_WIDTH;
return mode === "phone" ? Math.min(configuredWidth, MAX_WIDTH()) : width;
};
Comment on lines 505 to 508
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 23, 2026

Choose a reason for hiding this comment

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

P2 getWidth() inconsistency in tab mode

For mode === "phone", getWidth() correctly re-evaluates Math.min(configuredWidth, MAX_WIDTH()). However for tab mode it returns the closed-over width local variable. After a window resize event onWindowResize updates the local innerWidth and hides the sidebar, but it does not re-clamp width. On the next show() call setWidth(width) may use a stale value larger than the new MAX_WIDTH().

For consistency (and to avoid the stale-value issue), tab mode should also be re-clamped:

Suggested change
$el.getWidth = function () {
const width = innerWidth * 0.7;
return mode === "phone" ? (width >= 350 ? 350 : width) : MIN_WIDTH;
const configuredWidth = appSettings.value.sidebarWidth || MIN_WIDTH;
return mode === "phone" ? Math.min(configuredWidth, MAX_WIDTH()) : width;
};
$el.getWidth = function () {
const configuredWidth = appSettings.value.sidebarWidth || MIN_WIDTH;
return Math.min(configuredWidth, MAX_WIDTH());
};


return $el;
Expand Down
3 changes: 1 addition & 2 deletions src/components/sidebar/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ body.no-animation {
position: fixed;
left: 0;
top: 0;
width: 70vw;
max-width: 350px;
width: 280px;
height: 100vh;
display: flex;
background-color: rgb(153, 153, 255);
Expand Down
1 change: 1 addition & 0 deletions src/lang/en-us.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,7 @@
"change theme": "Change Theme",
"documentation": "Documentation",
"open in terminal": "Open in Terminal",
"sidebar width": "Sidebar width",
"developer mode": "Developer Mode",
"info-developermode": "Enable developer tools (Eruda) for debugging plugins and inspecting app state. Inspector will be initialized on app start.",
"developer mode enabled": "Developer mode enabled. Use command palette to toggle inspector (Ctrl+Shift+I).",
Expand Down
1 change: 1 addition & 0 deletions src/lib/settings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ class Settings {
colorPreview: true,
maxRetryCount: 3,
showRetryToast: false,
sidebarWidth: 280,
showSideButtons: true,
showSponsorSidebarApp: true,
showAnnotations: false,
Expand Down
15 changes: 15 additions & 0 deletions src/settings/appSettings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,21 @@ export default function otherSettings() {
info: strings["settings-info-app-floating-button"],
category: categories.interface,
},
{
key: "sidebarWidth",
text: strings["sidebar width"] || "Sidebar width",
value: values.sidebarWidth,
prompt: strings["sidebar width"] || "Sidebar width (pixels)",
promptType: "number",
promptOptions: {
test(value) {
value = Number.parseInt(value);
return value >= 200 && value <= 1500;
},
},
Comment on lines +96 to +101
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 23, 2026

Choose a reason for hiding this comment

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

P1 Validation ceiling (1500px) is disconnected from the runtime cap

The promptOptions.test allows values up to 1500 px, but at runtime the sidebar is clamped to innerWidth - 40. On a typical mobile device (e.g. 400 px wide) the ceiling is ~360 px — far below 1500. A user who enters, say, 800 px will pass validation, the value 800 will be persisted to settings, but the sidebar will silently render at 360 px. The mismatch between the stored value and the rendered width will be confusing.

Consider deriving the max from screen dimensions at validation time, or at least document the runtime clamping in the info string so users understand why the displayed width may differ from what they entered:

Suggested change
promptOptions: {
test(value) {
value = Number.parseInt(value);
return value >= 200 && value <= 1500;
},
},
test(value) {
value = Number.parseInt(value);
const runtimeMax = window.innerWidth - 40;
return value >= 200 && value <= runtimeMax;
},

info: "Adjust the width of the sidebar (min 200px).",
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 23, 2026

Choose a reason for hiding this comment

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

P2 Hard-coded English info string

Every other settings item in this file uses a localized strings[...] lookup for its info field. This one uses a bare string literal "Adjust the width of the sidebar (min 200px).", which will not be translated when the user changes language. Add a corresponding key to en-us.json and look it up via strings:

Suggested change
info: "Adjust the width of the sidebar (min 200px).",
info: strings["info-sidebarWidth"] || "Adjust the width of the sidebar (min 200px).",

category: categories.interface,
},
{
key: "showSideButtons",
text: strings["show side buttons"],
Expand Down
Loading