-
Notifications
You must be signed in to change notification settings - Fork 71
Add collapsible sidebar and redesign app layout #1748
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.
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
base: develop
Are you sure you want to change the base?
Changes from all commits
25041e2
7fed7c7
81bbf2c
f784fc0
dae6800
f618d27
b8a52a3
c720cb1
b99de96
eb1792c
44b0e79
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,165 @@ | ||||||
| /** | ||||||
| * Manages sidebar open/close state for mobile and desktop and handles sidebar section expand/collapse. | ||||||
| * | ||||||
| * Desktop: sidebar visible by default, content shifts when closed | ||||||
| * Mobile: sidebar hidden by default, overlays content when opened | ||||||
| */ | ||||||
| export default { | ||||||
| MOBILE_BREAKPOINT: 768, | ||||||
| STORAGE_KEY: 'backpex-sidebar-open', | ||||||
|
|
||||||
| 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() | ||||||
| }, | ||||||
|
Comment on lines
+11
to
+42
|
||||||
|
|
||||||
| isDesktop () { | ||||||
| return window.innerWidth >= this.MOBILE_BREAKPOINT | ||||||
| }, | ||||||
|
|
||||||
| handleToggle () { | ||||||
| if (this.isDesktop()) { | ||||||
| this.desktopOpen = !this.desktopOpen | ||||||
| this.saveDesktopState() | ||||||
| } else { | ||||||
| this.mobileOpen = !this.mobileOpen | ||||||
| } | ||||||
| this.applyState() | ||||||
| }, | ||||||
|
|
||||||
| loadDesktopState () { | ||||||
| const stored = localStorage.getItem(this.STORAGE_KEY) | ||||||
| // Default to open if no stored value | ||||||
| return stored === null ? true : stored === 'true' | ||||||
| }, | ||||||
|
|
||||||
| saveDesktopState () { | ||||||
| localStorage.setItem(this.STORAGE_KEY, this.desktopOpen.toString()) | ||||||
| }, | ||||||
|
|
||||||
| closeMobile () { | ||||||
| this.mobileOpen = false | ||||||
| this.applyState() | ||||||
| }, | ||||||
|
|
||||||
| handleResize (event) { | ||||||
| if (event.matches) { | ||||||
| this.mobileOpen = false | ||||||
| } | ||||||
| this.applyState() | ||||||
| }, | ||||||
|
|
||||||
| handleKeydown (event) { | ||||||
| if (event.key === 'Escape' && this.mobileOpen && !this.isDesktop()) { | ||||||
| this.closeMobile() | ||||||
| } | ||||||
| }, | ||||||
|
|
||||||
| applyState () { | ||||||
| const isDesktop = this.isDesktop() | ||||||
| const sidebarVisible = isDesktop ? this.desktopOpen : this.mobileOpen | ||||||
|
|
||||||
| // Sidebar transform | ||||||
| this.sidebar.classList.toggle('-translate-x-full', !sidebarVisible) | ||||||
| this.sidebar.classList.toggle('translate-x-0', sidebarVisible) | ||||||
|
|
||||||
| // Main content margin (desktop only, uses CSS variable) | ||||||
| const showMargin = isDesktop && this.desktopOpen | ||||||
| this.main.classList.toggle('ml-(--sidebar-width)', showMargin) | ||||||
|
||||||
| this.main.classList.toggle('ml-(--sidebar-width)', showMargin) | |
| this.main.classList.toggle('ml-[var(--sidebar-width)]', showMargin) |
Copilot
AI
Feb 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
AI
Feb 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.