Skip to content

fix: Hide navigation bar during onboarding animation #2370

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
wants to merge 2 commits into
base: main
Choose a base branch
from
+7 −3
Open

fix: Hide navigation bar during onboarding animation #2370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7c7ba1f
prevent subscription ovveride
HarshMN2345 Sep 15, 2025
e8ff176
removed initialheader
HarshMN2345 Sep 15, 2025
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
6 changes: 5 additions & 1 deletion src/lib/layout/shell.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,11 @@
});
// subscriptions
isNewWizardStatusOpen.subscribe((value) => (showHeader = !value));
isNewWizardStatusOpen.subscribe((value) => {
if (showHeader) {
showHeader = !value;
}
});
Comment on lines +126 to +130
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Guarded inversion can leave the header hidden after the wizard closes; derive visibility instead of mutating the prop.

When the wizard opens, showHeader is set to false, but when it closes (value === false) this block does nothing if showHeader is already false. On non-onboarding routes (where the parent doesn’t re-drive showHeader), the header can remain hidden until a route change. This also introduces an avoidable manual subscription that isn’t unsubscribed.

Apply this diff to remove the manual subscription:

-    isNewWizardStatusOpen.subscribe((value) => {
-        if (showHeader) {
-            showHeader = !value;
-        }
-    });
+    // Removed: use a derived reactive flag instead (see below).

Then, introduce a derived flag and use it in markup (outside this hunk):

@@
     // reactive blocks
@@
+    // Drive header visibility from parent intent AND wizard state; no local prop mutation.
+    $: effectiveShowHeader = showHeader && !$isNewWizardStatusOpen;
@@
-    class:no-header={!showHeader}
+    class:no-header={!effectiveShowHeader}
@@
-    {#if showHeader}
+    {#if effectiveShowHeader}

Why: avoids sticky state, prevents flicker on onboarding routes (parent still passes showHeader=false), and removes the untracked subscription.

Verification (quick manual):

  • Navigate to any non-onboarding page, open and close the new wizard: header should reappear immediately after close.
  • Navigate within /console/onboarding/**: header should remain hidden throughout, regardless of wizard open/close.

🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE="src/lib/layout/shell.svelte"
echo "PWD: $(pwd)"
if [ -f "$FILE" ]; then
  echo "== File exists: $FILE =="
  echo "== Grep for relevant symbols (context 3) =="
  rg -n -C3 'isNewWizardStatusOpen|\$isNewWizardStatusOpen|showHeader|class:no-header' "$FILE" || true
  echo "== Showing file (first 400 lines) =="
  sed -n '1,400p' "$FILE" || true
else
  echo "File not found: $FILE"
  echo "== Searching repo for symbols =="
  rg -n -C3 'isNewWizardStatusOpen|\$isNewWizardStatusOpen|showHeader|class:no-header' || true
fi

Length of output: 11750

Derive header visibility instead of mutating the exported prop

Manual subscription at src/lib/layout/shell.svelte (lines ~126-130) mutates the exported showHeader and is never unsubscribed — this can leave the header hidden after the wizard closes. Remove the subscription and drive visibility from the parent prop + wizard store.

Apply these changes:

@@
-    isNewWizardStatusOpen.subscribe((value) => {
-        if (showHeader) {
-            showHeader = !value;
-        }
-    });
+    // Removed manual subscription; header visibility is derived instead.

Add a derived reactive flag and use it in markup:

@@
     // reactive blocks
@@
+    // Drive header visibility from parent intent AND wizard state; no local prop mutation.
+    $: effectiveShowHeader = showHeader && !$isNewWizardStatusOpen;
@@
-    class:no-header={!showHeader}
+    class:no-header={!effectiveShowHeader}
@@
-    {#if showHeader}
+    {#if effectiveShowHeader}

src/lib/layout/shell.svelte: fix required at the subscription (≈lines 126-130) and the header markup (class/no-header + {#if showHeader}).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
isNewWizardStatusOpen.subscribe((value) => {
if (showHeader) {
showHeader = !value;
}
});
// Removed manual subscription; header visibility is derived instead.
// Drive header visibility from parent intent AND wizard state; no local prop mutation.
$: effectiveShowHeader = showHeader && !$isNewWizardStatusOpen;
class:no-header={!effectiveShowHeader}
{#if effectiveShowHeader}

page.subscribe(({ url }) => {
$showSubNavigation = url.searchParams.get('openNavbar') === 'true';
Expand Down
4 changes: 2 additions & 2 deletions src/routes/(console)/+layout.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,8 +339,8 @@
!page.url.pathname.includes('/console/account') &&
!page.url.pathname.includes('/console/card') &&
!page.url.pathname.includes('/console/onboarding')}
showHeader={!page.url.pathname.includes('/console/onboarding/create-project')}
showFooter={!page.url.pathname.includes('/console/onboarding/create-project')}
showHeader={!page.url.pathname.includes('/console/onboarding')}
showFooter={!page.url.pathname.includes('/console/onboarding')}
Comment on lines +342 to +343
Copy link
Member

Choose a reason for hiding this comment

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

can be exported to a var since its the same check.

selectedProject={page.data?.project}>
<!-- <Header slot="header" />-->
<slot />
Expand Down