Add collapsible sidebar and redesign app layout by Flo0807 · Pull Request #1748 · naymspace/backpex

Flo0807 · 2026-01-08T11:14:29Z

collapsible_sidebar.mov

Flo0807 · 2026-01-08T11:33:26Z

As this MR handles persistent state via local storage, it will result in showing the sidebar for a short amount of time on page load.

I suggest that we work on a more generic "UI state" solution in a dedicated PR.

Copilot

Pull request overview

This pull request adds a collapsible sidebar and redesigns the application layout. The changes replace the previous drawer-based sidebar with a new implementation that supports both mobile overlay and desktop push-aside modes, with state persistence for the desktop view.

Changes:

Replaced BackpexSidebarSections hook with a new BackpexSidebar hook that manages both sidebar visibility and section expansion
Redesigned app shell layout to use a fixed sidebar with overlay on mobile and content-shifting on desktop
Moved branding component from topbar to sidebar (breaking change from topbar_branding to sidebar_branding)

Reviewed changes

Copilot reviewed 9 out of 11 changed files in this pull request and generated 18 comments.

Show a summary per file

File	Description
assets/js/hooks/_sidebar.js	New hook implementation combining sidebar visibility management with section expansion logic
assets/js/hooks/_sidebar_sections.js	Removed old sections-only hook
assets/js/hooks/index.js	Updated export to reference new BackpexSidebar hook
priv/static/js/backpex.esm.js	Compiled ESM bundle with new sidebar implementation
priv/static/js/backpex.cjs.js	Compiled CJS bundle with new sidebar implementation
lib/backpex/html/layout.ex	Redesigned app shell layout with fixed sidebar and renamed branding component
demo/lib/demo_web/components/layouts/admin.html.heex	Updated demo to use new sidebar structure with sidebar_branding
demo/assets/css/app.css	Added CSS custom property for sidebar width
priv/gettext/backpex.pot	Updated translation strings for simplified navigation labels

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-26T12:48:24Z

priv/static/js/backpex.cjs.js

  mounted() {
+    this.sidebar = document.getElementById("backpex-sidebar");
+    this.overlay = document.getElementById("backpex-sidebar-overlay");
+    this.main = document.getElementById("backpex-main");
+    this.toggleBtn = document.getElementById("backpex-sidebar-toggle");
+    this.mobileOpen = false;
+    this.desktopOpen = this.loadDesktopState();
+    this.applyState();
+    this.toggleBtn.addEventListener("click", () => this.handleToggle());
+    this.overlay.addEventListener("click", () => this.closeMobile());
+    this.mediaQuery = window.matchMedia(
+      `(min-width: ${this.MOBILE_BREAKPOINT}px)`
+    );
+    this.mediaQuery.addEventListener("change", (e) => this.handleResize(e));
+    document.addEventListener("keydown", (e) => this.handleKeydown(e));
    this.initializeSections();
  },
  updated() {
+    this.applyState();
    this.initializeSections();
  },
-  destroyed() {
-    const sections = this.el.querySelectorAll("[data-section-id]");
-    sections.forEach((section) => {
-      const toggle = section.querySelector("[data-menu-dropdown-toggle]");
-      toggle.removeEventListener("click", this.handleToggle.bind(this));
-    });
+  isDesktop() {
+    return window.innerWidth >= this.MOBILE_BREAKPOINT;
  },
-  hasContent(element) {
-    if (!element || element.children.length === 0) {
-      return false;
+  handleToggle() {
+    if (this.isDesktop()) {
+      this.desktopOpen = !this.desktopOpen;
+      this.saveDesktopState();
+    } else {
+      this.mobileOpen = !this.mobileOpen;
    }
-    for (const child of element.children) {
-      const childContent = child.querySelector("[data-menu-dropdown-content]");
-      if (childContent) {
-        if (this.hasContent(childContent)) {
-          return true;
-        }
-      } else {
-        return true;
-      }
+    this.applyState();
+  },
+  loadDesktopState() {
+    const stored = localStorage.getItem(this.STORAGE_KEY);


The hook attempts to access DOM elements that may not exist when the sidebar is empty. The sidebar, overlay, and toggle button are conditionally rendered in the template when @sidebar != [], but this code doesn't check if these elements exist before trying to use them. This will cause JavaScript errors and crashes when the sidebar slot is empty. Add null checks for all DOM element lookups and early return if the sidebar doesn't exist.

Copilot · 2026-02-26T12:48:24Z

priv/static/js/backpex.cjs.js

  mounted() {
+    this.sidebar = document.getElementById("backpex-sidebar");
+    this.overlay = document.getElementById("backpex-sidebar-overlay");
+    this.main = document.getElementById("backpex-main");
+    this.toggleBtn = document.getElementById("backpex-sidebar-toggle");
+    this.mobileOpen = false;
+    this.desktopOpen = this.loadDesktopState();
+    this.applyState();
+    this.toggleBtn.addEventListener("click", () => this.handleToggle());
+    this.overlay.addEventListener("click", () => this.closeMobile());
+    this.mediaQuery = window.matchMedia(
+      `(min-width: ${this.MOBILE_BREAKPOINT}px)`
+    );
+    this.mediaQuery.addEventListener("change", (e) => this.handleResize(e));
+    document.addEventListener("keydown", (e) => this.handleKeydown(e));
    this.initializeSections();
  },
  updated() {
+    this.applyState();
    this.initializeSections();
  },


Missing a destroyed() lifecycle hook to clean up event listeners. The hook adds several event listeners in mounted() including click handlers, media query listeners, and a document keydown listener. Without proper cleanup, these listeners will persist even after the component is destroyed, leading to memory leaks and potential errors. Other hooks in this codebase consistently implement destroyed() to clean up resources.

Copilot · 2026-02-26T12:48:24Z

lib/backpex/html/layout.ex

+        :if={@sidebar != []}
+        id="backpex-sidebar"
+        class={[
+          "fixed inset-y-0 left-0 z-40 flex w-(--sidebar-width) flex-col",


The CSS class syntax w-(--sidebar-width) appears incorrect for Tailwind CSS. The standard way to use CSS custom properties in Tailwind arbitrary values is w-[var(--sidebar-width)]. The parentheses syntax used here may not be recognized by Tailwind's CSS processor and could result in the width not being applied correctly. Please verify this syntax works with your version of Tailwind CSS v4, or use the standard arbitrary value syntax with square brackets.

Suggested change

"fixed inset-y-0 left-0 z-40 flex w-(--sidebar-width) flex-col",

"fixed inset-y-0 left-0 z-40 flex w-[var(--sidebar-width)] flex-col",

Copilot · 2026-02-26T12:48:25Z

assets/js/hooks/_sidebar.js

+      toggle.removeEventListener('click', toggle._handler)
+      toggle._handler = (e) => this.handleSectionToggle(e)
+      toggle.addEventListener('click', toggle._handler)


Using toggle._handler to store the handler reference is a code smell. While this pattern works, it's unconventional and stores arbitrary properties on DOM elements. A cleaner approach would be to use a WeakMap to associate handlers with elements, or store handlers in a Map on the hook instance. This current approach could potentially conflict with other code that might use the same property name.

Copilot · 2026-02-26T12:48:25Z

assets/js/hooks/_sidebar.js

+  mounted () {
+    this.sidebar = document.getElementById('backpex-sidebar')
+    this.overlay = document.getElementById('backpex-sidebar-overlay')
+    this.main = document.getElementById('backpex-main')
+    this.toggleBtn = document.getElementById('backpex-sidebar-toggle')
+
+    // State: mobile closed by default, desktop state from localStorage (default open)
+    this.mobileOpen = false
+    this.desktopOpen = this.loadDesktopState()
+
+    // Apply initial state (CSS sets visible by default, JS hides on mobile)
+    this.applyState()
+
+    // Event listeners
+    this.toggleBtn.addEventListener('click', () => this.handleToggle())
+    this.overlay.addEventListener('click', () => this.closeMobile())
+
+    this.mediaQuery = window.matchMedia(
+      `(min-width: ${this.MOBILE_BREAKPOINT}px)`
+    )
+    this.mediaQuery.addEventListener('change', (e) => this.handleResize(e))
+
+    document.addEventListener('keydown', (e) => this.handleKeydown(e))
+
+    // Initialize sidebar sections
+    this.initializeSections()
+  },
+
+  updated () {
+    this.applyState()
+    this.initializeSections()
+  },


Missing a destroyed() lifecycle hook to clean up event listeners. The hook adds several event listeners in mounted() including click handlers, media query listeners, and a document keydown listener. Without proper cleanup, these listeners will persist even after the component is destroyed, leading to memory leaks and potential errors. Other hooks in this codebase consistently implement destroyed() to clean up resources.

Copilot · 2026-02-26T12:48:27Z

priv/static/js/backpex.esm.js

+    this.sidebar.classList.toggle("-translate-x-full", !sidebarVisible);
+    this.sidebar.classList.toggle("translate-x-0", sidebarVisible);
+    const showMargin = isDesktop && this.desktopOpen;
+    this.main.classList.toggle("ml-(--sidebar-width)", showMargin);


The CSS class syntax ml-(--sidebar-width) appears incorrect for Tailwind CSS. The standard way to use CSS custom properties in Tailwind arbitrary values is ml-[var(--sidebar-width)]. The parentheses syntax used here may not be recognized by Tailwind's CSS processor and could result in the class not applying the expected margin. Please verify this syntax works with your version of Tailwind CSS v4, or use the standard arbitrary value syntax with square brackets.

Suggested change

this.main.classList.toggle("ml-(--sidebar-width)", showMargin);

this.main.classList.toggle("ml-[var(--sidebar-width)]", showMargin);

Copilot · 2026-02-26T12:48:27Z

lib/backpex/html/layout.ex

@@ -269,16 +283,16 @@ defmodule Backpex.HTML.Layout do

  slot :logo, doc: "the logo of the branding"

-  def topbar_branding(assigns) do
+  def sidebar_branding(assigns) do


The removal of the topbar_branding component and its replacement with sidebar_branding is a breaking change. The documentation in guides/get_started/installation.md still references topbar_branding in multiple places (lines 143 and 576), which will cause errors for users following the guide. The documentation needs to be updated to reflect this change, showing the new sidebar_branding placement within the sidebar slot instead of the topbar slot.

Copilot · 2026-02-26T12:48:27Z

priv/static/js/backpex.cjs.js

+      toggle.removeEventListener("click", toggle._handler);
+      toggle._handler = (e) => this.handleSectionToggle(e);
+      toggle.addEventListener("click", toggle._handler);


Using toggle._handler to store the handler reference is a code smell. While this pattern works, it's unconventional and stores arbitrary properties on DOM elements. A cleaner approach would be to use a WeakMap to associate handlers with elements, or store handlers in a Map on the hook instance. This current approach could potentially conflict with other code that might use the same property name.

Suggested change

toggle.removeEventListener("click", toggle._handler);

toggle._handler = (e) => this.handleSectionToggle(e);

toggle.addEventListener("click", toggle._handler);

if (!this._sectionToggleHandlers) {

this._sectionToggleHandlers = new WeakMap();

}

const existingHandler = this._sectionToggleHandlers.get(toggle);

if (existingHandler) {

toggle.removeEventListener("click", existingHandler);

}

const handler = (e) => this.handleSectionToggle(e);

this._sectionToggleHandlers.set(toggle, handler);

toggle.addEventListener("click", handler);

Copilot · 2026-02-26T12:48:28Z

priv/static/js/backpex.cjs.js

+  applyState() {
+    const isDesktop = this.isDesktop();
+    const sidebarVisible = isDesktop ? this.desktopOpen : this.mobileOpen;
+    this.sidebar.classList.toggle("-translate-x-full", !sidebarVisible);
+    this.sidebar.classList.toggle("translate-x-0", sidebarVisible);
+    const showMargin = isDesktop && this.desktopOpen;
+    this.main.classList.toggle("ml-(--sidebar-width)", showMargin);
+    this.main.classList.toggle("ml-0", !showMargin);
+    const showOverlay = !isDesktop && this.mobileOpen;
+    this.overlay.classList.toggle("opacity-0", !showOverlay);
+    this.overlay.classList.toggle("pointer-events-none", !showOverlay);
+    this.overlay.classList.toggle("opacity-100", showOverlay);
+    this.overlay.classList.toggle("pointer-events-auto", showOverlay);
+    this.toggleBtn.setAttribute("aria-expanded", sidebarVisible.toString());
  },


Accessing properties on potentially null elements. If the sidebar doesn't exist, this.sidebar, this.overlay, this.main, and this.toggleBtn will be null, and calling methods like classList.toggle() on them will throw errors. This method is called from multiple places including updated() lifecycle, which can execute repeatedly.

Copilot · 2026-02-26T12:48:28Z

priv/static/js/backpex.cjs.js

+    this.sidebar.classList.toggle("-translate-x-full", !sidebarVisible);
+    this.sidebar.classList.toggle("translate-x-0", sidebarVisible);
+    const showMargin = isDesktop && this.desktopOpen;
+    this.main.classList.toggle("ml-(--sidebar-width)", showMargin);


The CSS class syntax ml-(--sidebar-width) appears incorrect for Tailwind CSS. The standard way to use CSS custom properties in Tailwind arbitrary values is ml-[var(--sidebar-width)]. The parentheses syntax used here may not be recognized by Tailwind's CSS processor and could result in the class not applying the expected margin. Please verify this syntax works with your version of Tailwind CSS v4, or use the standard arbitrary value syntax with square brackets.

Suggested change

this.main.classList.toggle("ml-(--sidebar-width)", showMargin);

this.main.classList.toggle("ml-[var(--sidebar-width)]", showMargin);

Flo0807 added 4 commits January 8, 2026 12:09

Implement collapsible sidebar

25041e2

Delete BackpexSidebar hook

7fed7c7

Update build

81bbf2c

Save sidebar state in local storage

f784fc0

Flo0807 self-assigned this Jan 8, 2026

Flo0807 marked this pull request as draft January 8, 2026 11:14

Flo0807 added 2 commits January 8, 2026 12:16

Format

dae6800

Update translations

f618d27

Flo0807 added 5 commits January 8, 2026 12:33

Update build

b8a52a3

Call applyState in updated hook

c720cb1

Rename topbar_branding into sidebar_branding

b99de96

Use CSS variable for sidebar width

eb1792c

Build assets

44b0e79

Flo0807 requested a review from Copilot February 26, 2026 12:41

Copilot started reviewing on behalf of Flo0807 February 26, 2026 12:41 View session

Copilot AI reviewed Feb 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add collapsible sidebar and redesign app layout#1748

Add collapsible sidebar and redesign app layout#1748
Flo0807 wants to merge 11 commits intodevelopfrom
feature/collapsible-sidebar

Flo0807 commented Jan 8, 2026 •

edited

Loading

Uh oh!

Flo0807 commented Jan 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	"fixed inset-y-0 left-0 z-40 flex w-(--sidebar-width) flex-col",
	"fixed inset-y-0 left-0 z-40 flex w-[var(--sidebar-width)] flex-col",

	this.main.classList.toggle("ml-(--sidebar-width)", showMargin);
	this.main.classList.toggle("ml-[var(--sidebar-width)]", showMargin);

-      toggle.removeEventListener("click", toggle._handler);
-      toggle._handler = (e) => this.handleSectionToggle(e);
-      toggle.addEventListener("click", toggle._handler);
+      if (!this._sectionToggleHandlers) {
+        this._sectionToggleHandlers = new WeakMap();
+      }
+      const existingHandler = this._sectionToggleHandlers.get(toggle);
+      if (existingHandler) {
+        toggle.removeEventListener("click", existingHandler);
+      }
+      const handler = (e) => this.handleSectionToggle(e);
+      this._sectionToggleHandlers.set(toggle, handler);
+      toggle.addEventListener("click", handler);

Conversation

Flo0807 commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Flo0807 commented Jan 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Flo0807 commented Jan 8, 2026 •

edited

Loading