Skip to content
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

Svelte 5 migration #160

Closed
wants to merge 2 commits into from
Closed

Svelte 5 migration #160

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
3571484
Add tbody in table to prevent svelte 5 migration error
evlim Oct 27, 2024
67a95da
migration to Svelte 5
evlim Oct 27, 2024
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
589 changes: 209 additions & 380 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

16 changes: 8 additions & 8 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
},
"devDependencies": {
"@sveltejs/adapter-auto": "^3.2.0",
"@sveltejs/kit": "^2.0.0",
"@sveltejs/vite-plugin-svelte": "^3.1.1",
"@sveltejs/kit": "^2.5.27",
"@sveltejs/vite-plugin-svelte": "^4.0.0",
"@tailwindcss/typography": "^0.5.13",
"@types/glob": "^8.1.0",
"@types/html-to-text": "^9.0.4",
Expand All @@ -28,19 +28,19 @@
"daisyui": "^4.7.3",
"eslint": "^8.28.0",
"eslint-config-prettier": "^9.1.0",
"eslint-plugin-svelte": "^2.30.0",
"eslint-plugin-svelte": "^2.45.1",
"fuse.js": "^7.0.0",
"glob": "^10.4.5",
"html-to-text": "^9.0.5",
"jsdom": "^24.1.1",
"postcss": "^8.4.31",
"prettier": "^3.0.3",
"prettier-plugin-svelte": "^3.0.3",
"prettier": "^3.1.0",
"prettier-plugin-svelte": "^3.2.6",
"super-sitemap": "^0.15.1",
"svelte": "^4.2.19",
"svelte-check": "^3.4.3",
"svelte": "^5.0.0",
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Migration to Svelte 5 requires several code updates

Based on the codebase analysis, there are multiple areas that need attention for the Svelte 5 migration:

  • Several files use $: reactive declarations that should be migrated to $derived
  • Multiple instances of $state() are already being used, showing partial migration to Svelte 5 runes
  • Lifecycle methods (onMount) are used in multiple components that need review:
    • src/routes/(marketing)/login/sign_in/+page.svelte
    • src/routes/(marketing)/search/+page.svelte
    • src/routes/(admin)/account/+layout.svelte
    • src/routes/(admin)/account/sign_out/+page.svelte
  • Store usage (writable) in src/routes/(admin)/account/(menu)/+layout.svelte needs to be migrated to $state
  • Several bind: directives that need compatibility verification
🔗 Analysis chain

⚠️ Consider the implications of upgrading to Svelte 5.

The upgrade to Svelte 5 (^5.0.0) represents a significant change as it's currently in beta. This version introduces major changes like the runes syntax and a new reactivity model.

Run this check to identify potential breaking changes:

Let me gather more information about the Svelte 5 migration readiness by checking for other potential breaking changes.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for Svelte 4 syntax that might need updates for Svelte 5
# Look for reactive statements and stores that might need migration to runes

# Check for $: reactive statements
echo "Checking for reactive statements that might need migration to $derived..."
rg -g '*.svelte' '\$: ' -A 2

# Check for writable store creation
echo "Checking for writable stores that might need migration to $state..."
rg -g '*.svelte' 'writable\(' -A 2

Length of output: 1439

Script:

#!/bin/bash
# Check for Svelte 5 migration concerns

# Check for lifecycle methods that might need updates
echo "Checking for lifecycle methods..."
rg -g '*.svelte' 'onMount|beforeUpdate|afterUpdate|onDestroy' -A 2

# Check for reactive declarations and computed values
echo "Checking for reactive declarations..."
rg -g '*.svelte' '\$: ' -A 2

# Check for store subscriptions that might need updates
echo "Checking for store subscriptions..."
rg -g '*.svelte' '\$[a-zA-Z]' -A 2

# Check for bind: directives that might need review
echo "Checking for bind directives..."
rg -g '*.svelte' 'bind:' -A 2

# Check for custom stores
echo "Checking for custom store implementations..."
rg -g '*.{js,ts}' 'function .+Store|class .+Store'

Length of output: 17828

"svelte-check": "^4.0.0",
"tailwindcss": "^3.4.1",
"typescript": "^5.0.0",
"typescript": "^5.5.0",
"vite": "^5.4.8",
"vitest": "^1.0.0"
},
Expand Down
325 changes: 167 additions & 158 deletions src/lib/emails/welcome_email_html.svelte
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 5 additions & 1 deletion src/lib/emails/welcome_email_text.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@
export const ssr = true

// Define all your props here
export let companyName: string = ""
interface Props {
companyName?: string;
}

let { companyName = "" }: Props = $props();
</script>

Welcome to {companyName}!
Expand Down
17 changes: 11 additions & 6 deletions src/routes/(admin)/account/(menu)/+layout.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,15 @@
import { writable } from "svelte/store"
import { setContext } from "svelte"
import { WebsiteName } from "../../../../config"
interface Props {
children?: import('svelte').Snippet;
}

let { children }: Props = $props();

const adminSectionStore = writable("")
setContext("adminSection", adminSectionStore)
let adminSection: string
let adminSection: string = $state()
adminSectionStore.subscribe((value) => {
adminSection = value
})
Comment on lines 12 to 17
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider simplifying state management

With Svelte 5's new reactivity system using $state(), the store subscription might be redundant. Consider refactoring to use either the store or $state(), but not both.

-const adminSectionStore = writable("")
-setContext("adminSection", adminSectionStore)
-let adminSection: string = $state()
-adminSectionStore.subscribe((value) => {
-  adminSection = value
-})
+let adminSection: string = $state("")
+setContext("adminSection", { subscribe: () => adminSection })

Committable suggestion was skipped due to low confidence.

Expand Down Expand Up @@ -47,12 +52,12 @@
</div>
</div>
<div class="container px-6 lg:px-12 py-3 lg:py-6">
<slot />
{@render children?.()}
</div>
</div>

<div class="drawer-side">
<label for="admin-drawer" class="drawer-overlay" />
<label for="admin-drawer" class="drawer-overlay"></label>
<ul
class="menu menu-lg p-4 w-80 min-h-full bg-base-100 lg:border-r text-primary"
>
Expand All @@ -68,7 +73,7 @@
<a
href="/account"
class={adminSection === "home" ? "active" : ""}
on:click={closeDrawer}
onclick={closeDrawer}
>
<svg
xmlns="http://www.w3.org/2000/svg"
Expand All @@ -90,7 +95,7 @@
<a
href="/account/billing"
class={adminSection === "billing" ? "active" : ""}
on:click={closeDrawer}
onclick={closeDrawer}
>
<svg
class="h-5 w-5"
Expand All @@ -109,7 +114,7 @@
<a
href="/account/settings"
class={adminSection === "settings" ? "active" : ""}
on:click={closeDrawer}
onclick={closeDrawer}
>
<svg class="h-5 w-5" viewBox="0 0 24 24" stroke="none" fill="none">
<g id="Interface / Settings">
Expand Down
6 changes: 5 additions & 1 deletion src/routes/(admin)/account/(menu)/billing/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@
let adminSection: Writable<string> = getContext("adminSection")
adminSection.set("billing")

export let data
interface Props {
data: any;
}

let { data }: Props = $props();
Comment on lines +14 to +18
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance type safety by properly typing the data prop.

While the migration to Svelte 5's $props() is correct, using any type reduces type safety. The component clearly expects specific properties on the data object.

Consider replacing the current interface with a more specific type definition:

 interface Props {
-  data: any;
+  data: {
+    currentPlanId?: string;
+    isActiveCustomer: boolean;
+    hasEverHadSubscription: boolean;
+  };
 }
📝 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
interface Props {
data: any;
}
let { data }: Props = $props();
interface Props {
data: {
currentPlanId?: string;
isActiveCustomer: boolean;
hasEverHadSubscription: boolean;
};
}
let { data }: Props = $props();


let currentPlanId = data.currentPlanId ?? defaultPlanId
let currentPlanName = pricingPlans.find(
Expand Down
6 changes: 5 additions & 1 deletion src/routes/(admin)/account/(menu)/settings/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
let adminSection: Writable<string> = getContext("adminSection")
adminSection.set("settings")

export let data
interface Props {
data: any;
}
Comment on lines +9 to +11
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider enhancing type safety by properly typing the data prop.

The any type reduces TypeScript's ability to catch potential errors. Based on the usage, we can define more specific types:

+interface Profile {
+  full_name?: string;
+  company_name?: string;
+  website?: string;
+  unsubscribed?: boolean;
+}
+
+interface User {
+  email?: string;
+}
+
 interface Props {
-  data: any;
+  data: {
+    profile: Profile;
+    user: User;
+  };
 }

Committable suggestion was skipped due to low confidence.


let { data }: Props = $props();
let { profile, user } = data
</script>

Expand Down
6 changes: 5 additions & 1 deletion src/routes/(admin)/account/(menu)/settings/change_email/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
let adminSection: Writable<string> = getContext("adminSection")
adminSection.set("settings")

export let data
interface Props {
data: any;
}
Comment on lines +9 to +11
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance type safety by properly typing the data prop.

The any type defeats TypeScript's type safety benefits. Consider defining a more specific interface structure since we know the data contains a user object.

 interface Props {
-  data: any;
+  data: {
+    user: {
+      email: string;
+      // Add other user properties as needed
+    };
+  };
 }

Committable suggestion was skipped due to low confidence.


let { data }: Props = $props();

let { user } = data
</script>
Expand Down
6 changes: 5 additions & 1 deletion src/routes/(admin)/account/(menu)/settings/change_email_subscription/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
<script lang="ts">
import SettingsModule from "../settings_module.svelte"
export let data
interface Props {
data: any;
}
Comment on lines +3 to +5
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Strengthen type safety by properly typing the Props interface.

Instead of using any, define a more specific type structure for the data property based on its usage in the component.

 interface Props {
-  data: any;
+  data: {
+    profile: {
+      unsubscribed?: boolean;
+      [key: string]: unknown;
+    };
+  };
 }
📝 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
interface Props {
data: any;
}
interface Props {
data: {
profile: {
unsubscribed?: boolean;
[key: string]: unknown;
};
};
}


let { data }: Props = $props();
let { profile } = data
let unsubscribed = profile?.unsubscribed
Comment on lines +7 to 9
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider improving null safety and destructuring.

While the migration to Svelte 5's $props() is correct, we can improve the code's robustness.

- let { data }: Props = $props();
- let { profile } = data
- let unsubscribed = profile?.unsubscribed
+ let { data: { profile } }: Props = $props();
+ let unsubscribed = profile?.unsubscribed ?? false

This change:

  1. Simplifies the nested destructuring
  2. Provides a default value for unsubscribed, ensuring consistent behavior
📝 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
let { data }: Props = $props();
let { profile } = data
let unsubscribed = profile?.unsubscribed
let { data: { profile } }: Props = $props();
let unsubscribed = profile?.unsubscribed ?? false

</script>
Expand Down
12 changes: 8 additions & 4 deletions src/routes/(admin)/account/(menu)/settings/change_password/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@
let adminSection: Writable<string> = getContext("adminSection")
adminSection.set("settings")

export let data
interface Props {
data: any;
}

let { data }: Props = $props();
Comment on lines +10 to +14
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider strengthening type definitions.

The Props interface uses any type which reduces type safety. Consider defining a more specific type structure based on the actual data shape.

 interface Props {
-  data: any;
+  data: {
+    user: {
+      email?: string;
+      amr?: Array<{ method: string }>;
+    };
+    supabase: SupabaseClient;
+  };
 }

Committable suggestion was skipped due to low confidence.

let { user, supabase } = data

// True if definitely has a password, but can be false if they
Expand All @@ -21,8 +25,8 @@
// @ts-expect-error: we ignore because Supabase does not maintain an AMR typedef
let usingOAuth = user?.amr?.find((x) => x.method === "oauth") ? true : false

let sendBtn: HTMLButtonElement
let sentEmail = false
let sendBtn: HTMLButtonElement = $state()
let sentEmail = $state(false)
let sendForgotPassword = () => {
sendBtn.disabled = true
sendBtn.textContent = "Sending..."
Expand Down Expand Up @@ -95,7 +99,7 @@
<button
class="btn btn-outline btn-wide {sentEmail ? 'hidden' : ''}"
bind:this={sendBtn}
on:click={sendForgotPassword}
onclick={sendForgotPassword}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Revert to Svelte's event binding syntax.

The change from on:click to onclick is incorrect. Svelte uses on:click for event binding, while onclick is the DOM attribute style which should be avoided in Svelte components.

-        onclick={sendForgotPassword}
+        on:click={sendForgotPassword}
📝 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
onclick={sendForgotPassword}
on:click={sendForgotPassword}

>Send Set Password Email
</button>
<div class="success alert alert-success {sentEmail ? '' : 'hidden'}">
Expand Down
6 changes: 5 additions & 1 deletion src/routes/(admin)/account/(menu)/settings/delete_account/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
let adminSection: Writable<string> = getContext("adminSection")
adminSection.set("settings")

export let data
interface Props {
data: any;
}
Comment on lines +9 to +11
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance type safety by defining a more specific Props interface.

The any type for data reduces TypeScript's ability to catch potential errors. Consider defining a more specific type structure based on the known shape of the data.

 interface Props {
-  data: any;
+  data: {
+    session: {
+      user: {
+        email: string;
+      } | null;
+    } | null;
+  };
 }
📝 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
interface Props {
data: any;
}
interface Props {
data: {
session: {
user: {
email: string;
} | null;
} | null;
};
}


let { data }: Props = $props();
let { session } = data
</script>

Expand Down
6 changes: 5 additions & 1 deletion src/routes/(admin)/account/(menu)/settings/edit_profile/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
let adminSection: Writable<string> = getContext("adminSection")
adminSection.set("settings")

export let data
interface Props {
data: any;
}
Comment on lines +9 to +11
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance type safety by properly typing the data structure.

Instead of using any, consider defining a more specific type structure for better type safety and code documentation:

 interface Props {
-  data: any;
+  data: {
+    profile?: {
+      full_name?: string;
+      company_name?: string;
+      website?: string;
+    };
+  };
 }
📝 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
interface Props {
data: any;
}
interface Props {
data: {
profile?: {
full_name?: string;
company_name?: string;
website?: string;
};
};
}


let { data }: Props = $props();

let { profile } = data
</script>
Expand Down
42 changes: 29 additions & 13 deletions src/routes/(admin)/account/(menu)/settings/settings_module.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
}

// Page state
let loading = false
let showSuccess = false
let loading = $state(false)
let showSuccess = $state(false)

type Field = {
inputType?: string // default is "text"
Expand All @@ -22,17 +22,33 @@
}

// Module context
export let editable = false
export let dangerous = false
export let title: string = ""
export let message: string = ""
export let fields: Field[]
export let formTarget: string = ""
export let successTitle = "Success"
export let successBody = ""
export let editButtonTitle: string | null = null
export let editLink: string | null = null
export let saveButtonTitle: string = "Save"
interface Props {
editable?: boolean;
dangerous?: boolean;
title?: string;
message?: string;
fields: Field[];
formTarget?: string;
successTitle?: string;
successBody?: string;
editButtonTitle?: string | null;
editLink?: string | null;
saveButtonTitle?: string;
}

let {
editable = false,
dangerous = false,
title = "",
message = "",
fields,
formTarget = "",
successTitle = "Success",
successBody = "",
editButtonTitle = null,
editLink = null,
saveButtonTitle = "Save"
}: Props = $props();

const handleSubmit: SubmitFunction = () => {
loading = true
Expand Down
17 changes: 13 additions & 4 deletions src/routes/(admin)/account/+layout.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
<script lang="ts">
import { run } from 'svelte/legacy';

import { invalidate } from "$app/navigation"
import { onMount } from "svelte"

export let data
interface Props {
data: any;
children?: import('svelte').Snippet;
}
Comment on lines +7 to +10
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider strengthening type safety for the data prop.

Using any type for the data prop reduces type safety. Consider creating a more specific type for the expected data structure.

 interface Props {
-  data: any;
+  data: {
+    supabase: SupabaseClient;
+    session: Session | null;
+  };
   children?: import('svelte').Snippet;
 }

Committable suggestion was skipped due to low confidence.


let { data, children }: Props = $props();

let { supabase, session } = data
$: ({ supabase, session } = data)
let { supabase, session } = $state(data)
run(() => {
({ supabase, session } = data)
});

onMount(() => {
const { data } = supabase.auth.onAuthStateChange((event, _session) => {
Expand All @@ -18,4 +27,4 @@
})
</script>

<slot />
{@render children?.()}
10 changes: 7 additions & 3 deletions src/routes/(admin)/account/create_profile/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@
import type { SubmitFunction } from "@sveltejs/kit"
import "../../../../app.css"

export let data
export let form: FormAccountUpdateResult
interface Props {
data: any;
form: FormAccountUpdateResult;
}
Comment on lines +6 to +9
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider improving type safety for the Props interface.

The data property is currently typed as any, which bypasses TypeScript's type checking benefits. Consider creating a more specific type that reflects the expected structure of { user: User, profile: Profile } based on its usage.

 interface Props {
-  data: any;
+  data: {
+    user: { email: string };
+    profile?: {
+      full_name?: string;
+      company_name?: string;
+      website?: string;
+    };
+  };
   form: FormAccountUpdateResult;
 }

Committable suggestion was skipped due to low confidence.


let { data, form }: Props = $props();

let { user, profile } = data

let loading = false
let loading = $state(false)
let fullName: string = profile?.full_name ?? ""
let companyName: string = profile?.company_name ?? ""
let website: string = profile?.website ?? ""
Expand Down
8 changes: 6 additions & 2 deletions src/routes/(admin)/account/sign_out/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@
import { goto } from "$app/navigation"
import { onMount } from "svelte"

export let data
interface Props {
data: any;
}
Comment on lines +5 to +7
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve type safety by properly typing the Props interface.

The any type defeats the purpose of TypeScript's type safety. Consider properly typing the data property to include the Supabase client type.

  interface Props {
-   data: any;
+   data: {
+     supabase: SupabaseClient;
+   };
  }

Committable suggestion was skipped due to low confidence.


let { data }: Props = $props();

let { supabase } = data
let message = "Signing out...."
let message = $state("Signing out....")

// on mount, sign out
onMount(() => {
Expand Down
10 changes: 6 additions & 4 deletions src/routes/(marketing)/+layout.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
<script>
import { WebsiteName } from "./../../config"
import "../../app.css"
/** @type {{children?: import('svelte').Snippet}} */
let { children } = $props();
</script>

<div class="navbar bg-base-100 container mx-auto">
Expand Down Expand Up @@ -34,8 +36,8 @@
</li>
</ul>
<div class="dropdown dropdown-end sm:hidden">
<!-- svelte-ignore a11y-label-has-associated-control -->
<!-- svelte-ignore a11y-no-noninteractive-tabindex -->
<!-- svelte-ignore a11y_label_has_associated_control -->
<!-- svelte-ignore a11y_no_noninteractive_tabindex -->
Comment on lines +39 to +40
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider addressing accessibility issues instead of ignoring them

While the comment format update is correct, consider addressing these accessibility concerns:

  1. For the label without control: Consider using aria-label or linking it to the menu content
  2. For the non-interactive tabindex: Consider making the element properly interactive or using a different pattern for the dropdown

Here's a potential improvement:

- <!-- svelte-ignore a11y_label_has_associated_control -->
- <!-- svelte-ignore a11y_no_noninteractive_tabindex -->
- <label tabindex="0" class="btn btn-ghost btn-circle">
+ <button 
+   aria-label="Menu"
+   aria-expanded="false"
+   aria-controls="mobile-menu"
+   class="btn btn-ghost btn-circle">

Also applies to: 56-56

<label tabindex="0" class="btn btn-ghost btn-circle">
<svg
xmlns="http://www.w3.org/2000/svg"
Expand All @@ -51,7 +53,7 @@
/></svg
>
</label>
<!-- svelte-ignore a11y-no-noninteractive-tabindex -->
<!-- svelte-ignore a11y_no_noninteractive_tabindex -->
<ul
tabindex="0"
class="menu menu-lg dropdown-content mt-3 z-[1] p-2 shadow bg-base-100 rounded-box w-52 font-bold"
Expand All @@ -72,7 +74,7 @@
</div>

<div class="">
<slot />
{@render children?.()}
</div>

<!-- Spacer grows so the footer can be at bottom on short pages -->
Expand Down
Loading
Loading