Skip to content

Fix: org mobile/tablet sidebar visibility and actions#2699

Merged
loks0n merged 1 commit intomainfrom
fix-SER-670-org-sidebar-visibilty-fix-for-mobile-tabs
Dec 11, 2025
Merged

Fix: org mobile/tablet sidebar visibility and actions#2699
loks0n merged 1 commit intomainfrom
fix-SER-670-org-sidebar-visibilty-fix-for-mobile-tabs

Conversation

@HarshMN2345
Copy link
Copy Markdown
Member

@HarshMN2345 HarshMN2345 commented Dec 11, 2025

What does this PR do?

Before

image

After

image

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)

Related PRs and Issues

(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)

Have you read the Contributing Guidelines on issues?

(Write your answer here.)

Summary by CodeRabbit

  • Bug Fixes
    • Improved sidebar visibility logic for tablet and smaller viewport sizes.
    • Fixed sidebar rendering conditions to properly show/hide based on navigation state and project selection.
    • Corrected sidebar visibility behavior when organization context is present.

✏️ Tip: You can customize this high-level summary in your review settings.

@appwrite
Copy link
Copy Markdown

appwrite Bot commented Dec 11, 2025

Console (appwrite/console)

Project ID: 688b7bf400350cbd60e9

Sites (1)
Site Status Logs Preview QR
 console-stage
688b7cf6003b1842c9dc
Ready Ready View Logs Preview URL QR Code

Tip

Trigger functions via HTTP, SDKs, events, webhooks, or scheduled cron jobs

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 11, 2025

Walkthrough

The pull request modifies sidebar and layout visibility logic across three components. Changes include: updating the viewport condition in sidebar.svelte from small to tablet sizes for rendering the bottom action area, introducing new reactive variables (shouldRenderSidebar and hasSidebarSpace) in shell.svelte to gate sidebar rendering and layout classes, and removing the organization parameter check from the side navigation visibility logic in the console layout file. These modifications consolidate sidebar visibility conditions and alter when the sidebar renders relative to viewport size and project context.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • shouldRenderSidebar and hasSidebarSpace interactions — Verify the new reactive variables correctly combine the conditions (wizard state, showSideNavigation, tablet viewport, selectedProject) and understand the intended sidebar visibility behavior across different states
  • Organization parameter removal — Assess the implications of no longer checking for organization parameters when controlling side navigation visibility, particularly for console routes and multi-tenant scenarios
  • Viewport condition change — Confirm that switching from isSmallViewport to isTabletViewport for the sidebar bottom action area aligns with the intended responsive design and doesn't inadvertently hide controls on small viewports
  • Layout class dependencies — Review the changes to icons-content and no-sidebar class logic to ensure proper layout rendering in all states (with/without sidebar, with/without project)

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix: org mobile/tablet sidebar visibility and actions' accurately and specifically describes the main changes across all modified files, which involve adjusting sidebar visibility logic for organization views on mobile and tablet viewports.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-SER-670-org-sidebar-visibilty-fix-for-mobile-tabs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/lib/layout/shell.svelte (1)

225-226: Align icons-content class with hasSidebarSpace for clearer layout logic

Right now icons-content is driven by state === 'icons' && selectedProject while no-sidebar is driven by !hasSidebarSpace. Since hasSidebarSpace already encodes “desktop + sidebar + project,” you could simplify and avoid duplicating conditions by tying icons-content to it as well, e.g.:

-        class:icons-content={state === 'icons' && selectedProject}
-        class:no-sidebar={!hasSidebarSpace}>
+        class:icons-content={hasSidebarSpace && state === 'icons'}
+        class:no-sidebar={!hasSidebarSpace}>

Functionally this should be equivalent for project pages and makes it easier to reason about layout purely in terms of hasSidebarSpace.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e714a46 and eabf5c0.

📒 Files selected for processing (3)
  • src/lib/components/sidebar.svelte (1 hunks)
  • src/lib/layout/shell.svelte (3 hunks)
  • src/routes/(console)/+layout.svelte (0 hunks)
💤 Files with no reviewable changes (1)
  • src/routes/(console)/+layout.svelte
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,svelte}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports

Files:

  • src/lib/layout/shell.svelte
  • src/lib/components/sidebar.svelte
**/*.{ts,tsx,js,jsx,svelte,json}

📄 CodeRabbit inference engine (AGENTS.md)

Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration

Files:

  • src/lib/layout/shell.svelte
  • src/lib/components/sidebar.svelte
**/*.svelte

📄 CodeRabbit inference engine (AGENTS.md)

Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development

Files:

  • src/lib/layout/shell.svelte
  • src/lib/components/sidebar.svelte
src/lib/components/**/*.svelte

📄 CodeRabbit inference engine (AGENTS.md)

Use PascalCase for component file names and place them in src/lib/components/[feature]/ directory structure

Files:

  • src/lib/components/sidebar.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build
  • GitHub Check: e2e
🔇 Additional comments (2)
src/lib/components/sidebar.svelte (1)

240-297: Tablet viewport condition for org-only action buttons

Switching the no-project branch to {:else if $isTabletViewport} means the Feedback/Support DropList stack now appears on both mobile and tablet widths for org views, while project views still gate their bottom action group on $isSmallViewport only. That asymmetry looks aligned with “org mobile/tablet” in the PR title, but it does mean tablets will show these actions for org pages but not for project pages.

Please sanity‑check org vs project behavior on small (<768px) and tablet (768–1023px) widths to confirm this matches UX expectations.

src/lib/layout/shell.svelte (1)

176-178: Sidebar gating via shouldRenderSidebar / hasSidebarSpace

Deriving shouldRenderSidebar from !$isNewWizardStatusOpen && showSideNavigation and then using it both to decide whether to render <Sidebar> and to compute hasSidebarSpace cleanly separates “should the nav exist” from “does the layout reserve space for it.” This also intentionally allows org pages (selectedProject falsy) with showSideNavigation = true to render a Sidebar while keeping hasSidebarSpace false so the content stays full‑width on desktop.

This looks consistent with the PR’s goal; just verify that no other routes set showSideNavigation = true without expecting a sidebar (or vice versa), so you don’t accidentally add a hidden-but-mounted Sidebar in unexpected places.

Also applies to: 209-218

@loks0n loks0n merged commit a56bf0b into main Dec 11, 2025
4 of 5 checks passed
@loks0n loks0n deleted the fix-SER-670-org-sidebar-visibilty-fix-for-mobile-tabs branch December 11, 2025 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants