Fix dropdown styling for <select> elements #1416
Conversation
WalkthroughThis PR standardizes component styling props (many Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-09-10T15:11:55.861ZApplied to files:
⏰ 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)
🔇 Additional comments (3)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/(authenticated)/projects/[id=idNumber]/forms/Authors.svelte (1)
108-162: Prevent form submission without author selection.The hidden input at line 110 will have an
undefinedvalue when no author is selected, allowing users to submit an incomplete form. Consider disabling the submit button whenselectedAuthoris null to improve UX and prevent invalid submissions.Apply this diff to disable the submit button when no author is selected:
- <BlockIfJobsUnavailable className="btn btn-primary"> + <BlockIfJobsUnavailable className="btn btn-primary {!selectedAuthor ? 'btn-disabled' : ''}"> {#snippet altContent()} {m.authors_submit()} {/snippet} - <button type="submit" class="btn btn-primary"> + <button type="submit" class="btn btn-primary" disabled={!selectedAuthor}> {m.authors_submit()} </button> </BlockIfJobsUnavailable>
🧹 Nitpick comments (1)
src/routes/(authenticated)/projects/[id=idNumber]/forms/Authors.svelte (1)
119-127: Use descriptive placeholder text instead of empty space.Line 122 uses a single space (
' ') as the fallback when no author is selected, which provides no guidance to users. Consider using a descriptive placeholder like an i18n message (e.g.,m.authors_select()or similar) to improve clarity.Example improvement:
<span class="px-1 w-full text-left"> {#if availableAuthors.length} - {selectedAuthor?.Name ?? ' '} + {selectedAuthor?.Name ?? m.authors_selectPlaceholder()} {:else} {m.authors_emptyGroup()} {/if} </span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(authenticated)/projects/[id=idNumber]/forms/Authors.svelte(3 hunks)
⏰ 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: checks / build-and-test
- GitHub Check: check / build-and-test
🔇 Additional comments (1)
src/routes/(authenticated)/projects/[id=idNumber]/forms/Authors.svelte (1)
48-60: LGTM! Clean state management approach.The superform integration is well-designed: removing the
formbinding since you're managing selection state locally, and theonUpdatedhandler appropriately resets the dropdown after form updates.
src/routes/(authenticated)/projects/[id=idNumber]/forms/Authors.svelte
Outdated
Show resolved
Hide resolved
|
Marking as draft for now. I will be working on a more thorough change that fixes the styling of all the |
Not including `class:` directive
I think this change happened in DaisyUI v5, but I can't locate the documentation of that change
FyreByrd
force-pushed
the
fix/author-select-ui
branch
from
d0e06e6 to
f4157cd
Compare
FyreByrd
changed the title
|
I found a fix based on daisyui source. It may not work in all browsers (i.e. may be only chromium-based), but I'm much happier with it than what I had after several days trying to create a usable Select component. |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/components/settings/LabeledFormInput.svelte (1)
10-14: Missingclassprop in Props interface.The
classprop is destructured on line 14 but not declared in thePropsinterface. This will cause a TypeScript error. Add theclassprop to the interface and importClassValue.<script lang="ts"> import type { Snippet } from 'svelte'; + import type { ClassValue } from 'svelte/elements'; import type { ValueKey } from '$lib/locales.svelte'; import { m } from '$lib/paraglide/messages'; interface Props extends ValueKey { + class?: ClassValue; children?: Snippet; } let { key, params = {}, class: classes, children }: Props = $props(); </script>
🧹 Nitpick comments (2)
src/lib/components/LocaleSelector.svelte (1)
74-75: Consider usingonkeydowninstead ofonkeypress.The
keypressevent is deprecated and doesn't fire for non-character keys. For better accessibility, useonkeydownwith a check for Enter/Space keys.onclick={() => onclick(locale)} - onkeypress={() => onclick(locale)} + onkeydown={(e) => (e.key === 'Enter' || e.key === ' ') && onclick(locale)}src/lib/components/SortTable.svelte (1)
86-86: Minor: unnecessary array wrapper.
ClassValuecan be passed directly without wrapping in an array, though this is functionally equivalent.- class={[c.class]} + class={c.class}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (50)
src/app.css(3 hunks)src/lib/components/BlockIfJobsUnavailable.svelte(1 hunks)src/lib/components/Dropdown.svelte(2 hunks)src/lib/components/LanguageCodeTypeahead.svelte(3 hunks)src/lib/components/LocaleSelector.svelte(2 hunks)src/lib/components/OrganizationDropdown.svelte(2 hunks)src/lib/components/Pagination.svelte(1 hunks)src/lib/components/SearchBar.svelte(2 hunks)src/lib/components/SortTable.svelte(4 hunks)src/lib/components/Tooltip.svelte(1 hunks)src/lib/components/TypeaheadInput.svelte(4 hunks)src/lib/components/settings/InputWithMessage.svelte(1 hunks)src/lib/components/settings/LabeledFormInput.svelte(1 hunks)src/lib/components/settings/PropertiesEditor.svelte(2 hunks)src/lib/components/settings/Toggle.svelte(2 hunks)src/lib/locales.svelte.ts(2 hunks)src/lib/products/components/ReleaseInfo.svelte(2 hunks)src/lib/projects/components/ProjectActionMenu.svelte(4 hunks)src/lib/projects/components/ProjectFilterSelector.svelte(1 hunks)src/routes/(authenticated)/+layout.svelte(6 hunks)src/routes/(authenticated)/admin/settings/build-engines/+page.svelte(1 hunks)src/routes/(authenticated)/admin/settings/organizations/edit/+page.svelte(2 hunks)src/routes/(authenticated)/admin/settings/organizations/new/+page.svelte(2 hunks)src/routes/(authenticated)/admin/settings/product-definitions/edit/+page.svelte(4 hunks)src/routes/(authenticated)/admin/settings/product-definitions/new/+page.svelte(4 hunks)src/routes/(authenticated)/admin/settings/stores/edit/+page.svelte(1 hunks)src/routes/(authenticated)/admin/settings/stores/new/+page.svelte(1 hunks)src/routes/(authenticated)/admin/settings/workflow-definitions/edit/+page.svelte(3 hunks)src/routes/(authenticated)/admin/settings/workflow-definitions/new/+page.svelte(4 hunks)src/routes/(authenticated)/directory/+page.svelte(2 hunks)src/routes/(authenticated)/organizations/[id=idNumber]/settings/products/+page.svelte(1 hunks)src/routes/(authenticated)/projects/[filter=projectSelector]/[[orgId=idNumber]]/+page.svelte(8 hunks)src/routes/(authenticated)/projects/[id=idNumber]/+page.svelte(2 hunks)src/routes/(authenticated)/projects/[id=idNumber]/ProductCard.svelte(2 hunks)src/routes/(authenticated)/projects/[id=idNumber]/edit/+page.svelte(4 hunks)src/routes/(authenticated)/projects/[id=idNumber]/forms/Authors.svelte(2 hunks)src/routes/(authenticated)/projects/[id=idNumber]/forms/OwnerGroup.svelte(2 hunks)src/routes/(authenticated)/projects/[id=idNumber]/forms/Reviewers.svelte(3 hunks)src/routes/(authenticated)/projects/[id=idNumber]/forms/valibot.ts(1 hunks)src/routes/(authenticated)/projects/[id=idNumber]/modals/DeleteProduct.svelte(1 hunks)src/routes/(authenticated)/projects/[id=idNumber]/modals/Properties.svelte(2 hunks)src/routes/(authenticated)/projects/import/[id=idNumber]/+page.svelte(2 hunks)src/routes/(authenticated)/projects/new/[id=idNumber]/+page.svelte(6 hunks)src/routes/(authenticated)/tasks/[product_id]/+page.svelte(9 hunks)src/routes/(authenticated)/users/[id=idNumber]/settings/profile/+page.svelte(4 hunks)src/routes/(authenticated)/users/invite/+page.svelte(2 hunks)src/routes/(authenticated)/users/org/[[orgId=idNumber]]/+page.svelte(2 hunks)src/routes/(authenticated)/workflow-instances/+page.svelte(5 hunks)src/routes/(authenticated)/workflow-instances/[product_id]/+page.svelte(1 hunks)src/routes/(unauthenticated)/downloads/apk/[package]/published/[email protected](1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/routes/(authenticated)/admin/settings/product-definitions/new/+page.svelte
- src/lib/components/Pagination.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(authenticated)/projects/[id=idNumber]/forms/Authors.svelte
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: 2025-09-12T14:07:02.200Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1283
File: src/routes/(authenticated)/tasks/[product_id]/+page.server.ts:285-303
Timestamp: 2025-09-12T14:07:02.200Z
Learning: In src/routes/(authenticated)/tasks/[product_id]/+page.server.ts, FyreByrd prefers to optimize filterAvailableActions by creating Sets at the caller level rather than inside the function, so the function would take Set<number> arguments instead of arrays for better performance and separation of concerns.
Applied to files:
src/routes/(authenticated)/organizations/[id=idNumber]/settings/products/+page.sveltesrc/routes/(authenticated)/workflow-instances/[product_id]/+page.sveltesrc/routes/(authenticated)/workflow-instances/+page.sveltesrc/routes/(authenticated)/projects/[id=idNumber]/modals/DeleteProduct.sveltesrc/routes/(authenticated)/tasks/[product_id]/+page.svelte
📚 Learning: 2025-09-10T15:11:55.861Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1275
File: src/routes/(authenticated)/projects/[id=idNumber]/+page.svelte:221-221
Timestamp: 2025-09-10T15:11:55.861Z
Learning: In Svelte 5, component props are declared using interface Props and destructured from $props(), not with the export let syntax used in Svelte 4. For example, instead of "export let canEdit: boolean = false;", Svelte 5 uses interface Props { canEdit: boolean; } and let { canEdit }: Props = $props();
Applied to files:
src/lib/components/settings/InputWithMessage.sveltesrc/routes/(authenticated)/projects/[id=idNumber]/modals/Properties.sveltesrc/lib/components/BlockIfJobsUnavailable.sveltesrc/routes/(authenticated)/projects/[id=idNumber]/edit/+page.sveltesrc/routes/(authenticated)/admin/settings/workflow-definitions/new/+page.sveltesrc/routes/(authenticated)/admin/settings/workflow-definitions/edit/+page.sveltesrc/lib/components/LanguageCodeTypeahead.sveltesrc/routes/(authenticated)/admin/settings/product-definitions/edit/+page.sveltesrc/lib/components/SearchBar.sveltesrc/routes/(authenticated)/users/[id=idNumber]/settings/profile/+page.sveltesrc/lib/components/Dropdown.sveltesrc/lib/components/settings/PropertiesEditor.sveltesrc/lib/components/TypeaheadInput.sveltesrc/lib/components/Tooltip.sveltesrc/lib/products/components/ReleaseInfo.sveltesrc/lib/components/settings/LabeledFormInput.svelte
📚 Learning: 2025-09-22T15:01:43.014Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1297
File: src/auth.ts:13-13
Timestamp: 2025-09-22T15:01:43.014Z
Learning: SvelteKit uses the devalue library for serialization, which supports Map and Set objects. This means Maps can be safely stored in session data and will be properly serialized/deserialized for client-side usage, unlike standard JSON serialization which would lose Map objects.
Applied to files:
src/lib/locales.svelte.ts
📚 Learning: 2025-10-02T01:36:08.124Z
Learnt from: 7dev7urandom
Repo: sillsdev/appbuilder-portal PR: 1318
File: src/lib/components/OrganizationDropdown.svelte:30-39
Timestamp: 2025-10-02T01:36:08.124Z
Learning: In the `src/lib/components/OrganizationDropdown.svelte` component, the `organizations` array will always contain at least one organization, so there's no need to handle the empty array case.
Applied to files:
src/routes/(authenticated)/directory/+page.sveltesrc/lib/components/OrganizationDropdown.sveltesrc/routes/(authenticated)/admin/settings/organizations/edit/+page.sveltesrc/routes/(authenticated)/admin/settings/organizations/new/+page.sveltesrc/routes/(authenticated)/+layout.svelte
📚 Learning: 2025-09-22T14:59:53.366Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1297
File: src/routes/(authenticated)/projects/[filter=projectSelector]/[orgId=idNumber]/+page.server.ts:214-221
Timestamp: 2025-09-22T14:59:53.366Z
Learning: In the AppBuilder portal, bulk project actions in the route `/projects/[filter=projectSelector]/[orgId=idNumber]/+page.server.ts` are organization-scoped by design. The route parameter `[orgId=idNumber]` ensures all operations happen within a single organization context, so using the route-level orgId for all projects in bulk actions like `doProjectAction` is correct and safe.
Applied to files:
src/routes/(authenticated)/projects/[filter=projectSelector]/[[orgId=idNumber]]/+page.svelte
📚 Learning: 2025-09-15T15:13:50.486Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1275
File: src/routes/(authenticated)/projects/[filter=projectSelector]/[id]/+page.svelte:202-241
Timestamp: 2025-09-15T15:13:50.486Z
Learning: In the project listing page, the `canArchiveSelected` permission check is intentionally used for both archive operations and rebuild/republish operations because these operations require the same authorization conditions.
Applied to files:
src/routes/(authenticated)/projects/[filter=projectSelector]/[[orgId=idNumber]]/+page.svelte
📚 Learning: 2025-09-17T19:48:21.249Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1275
File: src/routes/(authenticated)/projects/[id=idNumber]/forms/OwnerGroup.svelte:5-5
Timestamp: 2025-09-17T19:48:21.249Z
Learning: In SvelteKit 2.12 and later, `page` should be imported from `$app/state`, not `$app/stores`. The `$app/stores` module is deprecated in favor of `$app/state` for modern SvelteKit versions.
Applied to files:
src/routes/(authenticated)/admin/settings/stores/new/+page.svelte
📚 Learning: 2025-09-25T03:51:13.814Z
Learnt from: 7dev7urandom
Repo: sillsdev/appbuilder-portal PR: 1297
File: src/routes/(unauthenticated)/invitations/organization-membership/+page.server.ts:7-8
Timestamp: 2025-09-25T03:51:13.814Z
Learning: In the organization invitation flow in src/routes/(unauthenticated)/invitations/organization-membership/+page.server.ts, the checkInviteErrors function properly handles null/undefined invite tokens by accepting inviteToken?: string | null and having an explicit guard clause if (!inviteToken) return { error: 'not found' }, making the non-null assertion on searchParams.get('t')! safe since error handling is centralized.
Applied to files:
src/routes/(authenticated)/users/invite/+page.svelte
📚 Learning: 2025-09-17T21:07:18.139Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1275
File: src/lib/projects/index.ts:183-201
Timestamp: 2025-09-17T21:07:18.139Z
Learning: In the AppBuilder portal project, org admins must be in the project group to claim/take ownership of projects. This is enforced both in client-side logic (canClaimProject function) and server-side validation.
Applied to files:
src/lib/projects/components/ProjectActionMenu.svelte
📚 Learning: 2025-09-22T15:01:28.480Z
Learnt from: FyreByrd
Repo: sillsdev/appbuilder-portal PR: 1297
File: src/lib/projects/index.ts:185-197
Timestamp: 2025-09-22T15:01:28.480Z
Learning: In the AppBuilder portal project, there's a policy inconsistency in canClaimProject: the docstring says "AppBuilder or OrgAdmin roles" but the implementation only allows OrgAdmin (via canModifyProject → isAdminForOrg). The UI code suggests AppBuilder users should have some project modification rights. This needs clarification from the maintainer.
Applied to files:
src/lib/projects/components/ProjectActionMenu.svelte
🪛 Biome (2.1.2)
src/app.css
[error] 133-134: Unexpected unknown pseudo-elements: picker(select)
See MDN web docs for more details.
Use a known pseudo-elements instead, such as:
(lint/correctness/noUnknownPseudoElement)
🔇 Additional comments (84)
src/routes/(authenticated)/admin/settings/stores/new/+page.svelte (1)
52-52: LGTM! Deprecated class removed as intended.The removal of
select-borderedaligns with the PR objectives to eliminate deprecated DaisyUI classes. The select element retains all functional attributes and bindings.Based on the PR description, drop shadow and dark mode border styling should be applied via CSS changes in other files. Please verify that the corresponding CSS rules are present (e.g., in
app.css) to ensure this select element remains visually distinct after the border class is removed.src/lib/locales.svelte.ts (2)
1-1: LGTM: Appropriate use of Svelte's ClassValue type.The import aligns with Svelte 5 best practices for typed class properties.
9-13: Property rename fromclassestoclassis properly handled across the codebase.The refactor to align with Svelte conventions and increase flexibility with
ClassValuetype is complete. Verification confirms zero remaining usages of the old.classesproperty across the codebase, and all consuming code in DataDisplayBox, Toggle, ToggleForm, InputWithMessage, and LabeledFormInput has been correctly updated to use the new type.src/routes/(authenticated)/projects/[id=idNumber]/modals/Properties.svelte (3)
73-73: ThePropertiesEditorcomponent properly accepts theclassprop. The component definition declaresclass?: ClassValuein its Props interface (line 9) and correctly destructures and applies it to the textarea element (lines 13, 27). The usage on line 73 is correct and consistent with the component's implementation.
50-50: No issues found. TheLabeledFormInputcomponent accepts theclassprop, as shown in its definition at line 14 where the destructuring patternclass: classesenables externalclassprop usage. The change is consistent with the component's prop signature and matches the pattern used throughout the codebase.
52-52: Theselect-borderedremoval is correct and consistent with DaisyUI v5.The project uses DaisyUI v5, which removed the select-bordered class since select components now have a border by default. The codebase shows complete migration to v5 standards with no remaining
select-borderedusages, confirming this change aligns with the overall styling approach.src/routes/(unauthenticated)/downloads/apk/[package]/published/[email protected] (1)
56-59: Array syntax forclassattribute is supported and correct in Svelte 5.19.2.Since Svelte 5.16, class can be an object or array, and is converted to a string using clsx, meaning the array is properly converted to space-separated class names. No changes are required.
src/app.css (1)
40-40: LGTM! Shadow depth enabled for dropdown visibility.Changing
--depthfrom0to1in both themes enables DaisyUI's shadow system, which aligns with the PR objective to add drop shadows to select dropdowns for improved visibility.Also applies to: 74-74
src/routes/(authenticated)/users/invite/+page.svelte (2)
76-76: LGTM!The prop rename from
classNametoclassaligns with the updatedOrganizationDropdowncomponent API that now acceptsclass?: ClassValue.
121-121: LGTM!Consistent with the
BlockIfJobsUnavailablecomponent's updated API usingclass?: ClassValue.src/routes/(authenticated)/organizations/[id=idNumber]/settings/products/+page.svelte (1)
36-47: LGTM!The
Togglecomponent usage is updated correctly:
title.classreplacestitle.classesfor the nested stylingclass="pb-2"replaces the previousclassNamepropBoth changes align with the standardized
ClassValue-based styling API.src/routes/(authenticated)/admin/settings/stores/edit/+page.svelte (1)
54-58: LGTM — intentional removal ofselect-bordered.The removal of the
select-borderedclass aligns with the PR's stated objective to remove deprecated classes. The PR adds drop shadow and dark-mode border styling elsewhere to maintain visual affordance for select elements.src/lib/components/settings/InputWithMessage.svelte (2)
9-20: LGTM — clean migration toClassValue-based styling.The prop rename from
classNametoclasswithClassValuetype follows Svelte conventions. The destructuring pattern{ class: classes }correctly handles the reserved keyword.
23-34: TheValueKeytype insrc/lib/locales.svelte.tsalready includes an optionalclassproperty, so no TypeScript error will occur. No changes needed.Likely an incorrect or invalid review comment.
src/routes/(authenticated)/admin/settings/organizations/new/+page.svelte (2)
60-67: LGTM!The prop rename from
classNametoclassaligns with the updatedInputWithMessagecomponent API.
99-110: LGTM!Consistent usage of the
classprop matching theInputWithMessagecomponent's updated interface.src/lib/components/LocaleSelector.svelte (2)
1-14: LGTM!The module-level
getFlagexport is well-structured with a sensible UN flag fallback for unrecognized locales. This enables reuse elsewhere (e.g., Reviewers form).
26-48: LGTM!The Props interface with nested
classobject and function-basedcurrentLocaleprop provides a flexible API. The defaults ensure backward compatibility for the global locale selector use case.src/lib/components/settings/PropertiesEditor.svelte (1)
2-27: LGTM!Clean migration to the
ClassValue-basedclassprop with proper typing and array binding syntax.src/lib/components/LanguageCodeTypeahead.svelte (2)
107-124: LGTM!The consolidated
classprop with nesteddropdownandinputproperties provides a cleaner API than separatedropdownClasses/inputClassesprops. Optional chaining ensures graceful handling whenclassis undefined.
156-165: LGTM!The class prop correctly merges the component's base
pr-20padding with consumer-provided input classes, maintaining the typeahead's layout while allowing customization.src/lib/components/TypeaheadInput.svelte (2)
7-20: LGTM!The
classprop withdefaultanddropdownsub-properties provides a clean API for styling both the input and dropdown list independently.
77-82: LGTM!The array class binding with conditional
'hidden'class is a clean Svelte 5 pattern. Falsy values are automatically filtered out.src/routes/(authenticated)/users/org/[[orgId=idNumber]]/+page.svelte (1)
160-160: LGTM!The prop renames from
classNametoclassforBlockIfJobsUnavailableandSearchBarcorrectly align with the updated component APIs usingClassValue.Also applies to: 177-177
src/routes/(authenticated)/admin/settings/build-engines/+page.svelte (1)
24-24: LGTM!The
Tooltipprop rename fromclassNametoclassmatches the updated component API.src/routes/(authenticated)/admin/settings/organizations/edit/+page.svelte (1)
64-64: LGTM!The prop renames from
classNametoclassforInputWithMessageandTogglecorrectly align with the updated component APIs.Also applies to: 106-106
src/routes/(authenticated)/projects/[id=idNumber]/+page.svelte (1)
102-102: LGTM!The prop renames from
classNametoclassforTooltipandBlockIfJobsUnavailablecorrectly align with the updated component APIs.Also applies to: 196-196
src/routes/(authenticated)/projects/[id=idNumber]/modals/DeleteProduct.svelte (1)
96-96: LGTM!The
BlockIfJobsUnavailableprop rename fromclassNametoclassmatches the updated component API.src/lib/projects/components/ProjectFilterSelector.svelte (1)
25-29: LGTM! Clean migration to nested class prop.The refactor from separate string props to a single nested
classobject aligns well with the broader standardization effort across the codebase.src/routes/(authenticated)/workflow-instances/+page.svelte (4)
76-76: LGTM! Consistent prop naming.The migration from
classNametoclassaligns with Svelte's standard attribute naming and the broader refactor.
80-80: LGTM! Removal of deprecated class.Removing
select-borderedis consistent with the PR's objective to standardize select element styling.
110-110: LGTM! SortTable prop standardization.Consistent with the project-wide migration to the
classprop naming convention.Also applies to: 177-177
121-121: LGTM! Tooltip prop standardization.Consistent with the project-wide migration to the
classprop naming convention.Also applies to: 188-188
src/routes/(authenticated)/projects/[filter=projectSelector]/[[orgId=idNumber]]/+page.svelte (2)
197-197: LGTM! Tooltip prop standardization.Consistent with the project-wide migration to the
classprop naming convention.
217-217: LGTM! BlockIfJobsUnavailable prop standardization.All seven instances consistently updated from
classNametoclass, aligning with the component's updated API.Also applies to: 237-237, 257-257, 340-340, 362-362, 391-391, 402-402
src/routes/(authenticated)/projects/[id=idNumber]/ProductCard.svelte (3)
170-172: LGTM! Dropdown API migration.The refactor to a nested
classobject is consistent with the broader standardization effort.
177-177: LGTM! DaisyUI class update.Good catch updating the deprecated
menu-compacttomenu-sm, as noted in the PR description.
183-183: LGTM! BlockIfJobsUnavailable prop standardization.Consistent with the component's updated API using
classinstead ofclassName.Also applies to: 225-225
src/routes/(authenticated)/projects/import/[id=idNumber]/+page.svelte (3)
97-97: LGTM! LabeledFormInput prop standardization.All three instances consistently updated from
classestoclass.Also applies to: 104-104, 111-111
98-98: LGTM! Select styling cleanup.Removing
select-borderedis consistent with the PR's objective to standardize select element styling across the application.Also applies to: 105-105
172-172: LGTM! BlockIfJobsUnavailable prop standardization.Consistent with the component's updated API.
src/lib/components/BlockIfJobsUnavailable.svelte (2)
3-3: LGTM! Component API modernization.Migrating from
className?: stringtoclass?: ClassValueprovides better type safety and flexibility for class composition. The ClassValue type supports strings, arrays, and objects, making it more versatile than plain strings.Also applies to: 11-13
20-20: LGTM! Array-based class binding.The array syntax
class={[classes, 'opacity-30 cursor-not-allowed']}is clean and idiomatic for Svelte 5, properly composing the passed classes with the button's disabled styling.src/routes/(authenticated)/admin/settings/product-definitions/edit/+page.svelte (2)
61-61: LGTM! Select styling standardization.All four select elements consistently updated by removing
select-bordered, aligning with the PR's objective to standardize select element styling.Also applies to: 73-73, 81-81, 90-90
109-109: LGTM! PropertiesEditor prop standardization.Consistent with the project-wide migration to the
classprop naming convention.src/routes/(authenticated)/directory/+page.svelte (3)
73-73: LGTM: Prop renaming to standardizedclassAPI.The updates to use
classinstead ofclassNamefor OrganizationDropdown and Tooltip are consistent with the PR's goal of standardizing component styling props.Also applies to: 78-78
94-94: LGTM: Nested class object for LanguageCodeTypeahead.The update to use a nested class object (
class={{ input: '...' }}) properly aligns with the new structured API for component styling.
97-97: Verify select styling withoutselect-bordered.The
select-borderedclass has been removed per the PR's deprecation cleanup. Ensure that the select element maintains adequate visual affordance without the explicit border class, especially since the PR objectives mention improving dropdown visibility.src/lib/components/settings/Toggle.svelte (2)
14-14: LGTM: Prop renamed toclasswith proper destructuring.The renaming of
classNametoclassand the corresponding destructuring (class: classes) follows the PR's standardization pattern correctly.Also applies to: 25-25
30-36: LGTM: Array-based class binding with proper conditional logic.The migration to array-based class bindings is idiomatic Svelte 5 and correctly handles conditional classes for disabled states.
src/routes/(authenticated)/workflow-instances/[product_id]/+page.svelte (3)
139-139: LGTM: Nested class object for Dropdown.The Dropdown component now uses the structured class object API (
class={{ label: '...', content: '...' }}), properly aligning with the new component API.
144-144: LGTM: Updated tomenu-smfrom deprecatedmenu-compact.The change from
menu-compacttomenu-smaddresses the deprecation mentioned in the PR description.
151-151: LGTM: Prop renamed toclass.BlockIfJobsUnavailable now uses the standardized
classprop instead ofclassName.src/lib/projects/components/ProjectActionMenu.svelte (3)
55-59: LGTM: Nested class object for Dropdown.The structured class object properly defines dropdown, label, and content styling, aligning with the new Dropdown API.
70-70: LGTM: Updated tomenu-sm.Replaced deprecated
menu-compactwithmenu-smas intended by the PR.
73-73: LGTM: BlockIfJobsUnavailable prop standardization.All three instances correctly use the
classprop instead ofclassName.Also applies to: 86-86, 104-104
src/lib/components/SearchBar.svelte (1)
10-10: LGTM: Complete migration to ClassValue API.The component properly imports
ClassValue, updates the prop signature, destructures correctly, and uses array-based class binding. This is a clean migration to the standardized styling API.Also applies to: 15-15, 22-22, 37-37
src/lib/components/OrganizationDropdown.svelte (1)
2-2: LGTM: Clean ClassValue migration.The component properly imports
ClassValue, updates the prop signature fromclassName?: stringtoclass?: ClassValue, and uses array-based binding (class={['select', classes]}) to combine the base class with the provided classes. This is a complete and correct migration.Also applies to: 9-9, 17-17, 29-29
src/lib/components/Tooltip.svelte (1)
3-3: LGTM: Complete ClassValue migration for Tooltip.The component correctly adopts the
ClassValuetype and array-based binding pattern. The base classes including the dark mode variant (dark:tooltip-secondary) are properly preserved.Also applies to: 7-7, 12-12, 16-16
src/routes/(authenticated)/users/[id=idNumber]/settings/profile/+page.svelte (2)
117-120: LGTM: TypeaheadInput with nested class object.The nested class object properly specifies
defaultanddropdownkeys. The conditionalselect-errorclass is correctly applied using array syntax whentzValueis invalid.
130-130: LGTM: InputWithMessage prop standardization.All three instances correctly use the
classprop instead ofclassName, consistent with the PR's API standardization.Also applies to: 142-142, 153-153
src/routes/(authenticated)/projects/[id=idNumber]/edit/+page.svelte (4)
40-40: LGTM: Prop standardization.The migration from
classestoclassprop for LabeledFormInput is consistent with the broader API standardization across components.Also applies to: 50-50, 64-64, 71-71, 87-87
51-51: LGTM: API standardization.BlockIfJobsUnavailable prop updated to
classconsistently with the component refactor.
55-55: LGTM: Deprecated class removal.Removal of
select-borderedaligns with the PR objective to clean up deprecated DaisyUI classes.Also applies to: 65-65
72-84: LGTM: Class prop object structure.LanguageCodeTypeahead now uses a structured class object, which is more maintainable than separate string props.
src/routes/(authenticated)/projects/[id=idNumber]/forms/OwnerGroup.svelte (1)
99-142: LGTM: Dropdown API update.Both Dropdown components now use the structured class object with
labelandcontentfields, consistent with the component API refactor.Also applies to: 157-190
src/routes/(authenticated)/admin/settings/workflow-definitions/new/+page.svelte (2)
51-56: LGTM: Deprecated class removal.Removal of
select-borderedfrom all select elements aligns with the PR's cleanup of deprecated DaisyUI classes.Also applies to: 59-66, 69-74, 86-91, 94-99
102-109: LGTM: Prop standardization.Component props updated from
className/classestoclass, consistent with the broader API refactor.Also applies to: 110-121, 112-119
src/routes/(authenticated)/projects/[id=idNumber]/forms/Reviewers.svelte (2)
38-38: Verify dropdown overflow behavior.The removal of
overflow-hiddenfrom the card container may be intentional to prevent clipping the LocaleSelector dropdown, but verify this doesn't cause visual issues in other parts of the card.
96-114: LGTM: Enhanced locale selector with visual flags.The LocaleSelector component provides better UX with flag icons and aligns with the PR objective to add a styled selector for reviewer locale. The hidden input preserves form submission behavior.
src/routes/(authenticated)/projects/new/[id=idNumber]/+page.svelte (2)
53-62: LGTM: Consistent prop standardization.All component prop updates from
classes/classNametoclassand removal ofselect-borderedare consistent with the PR's standardization objectives.Also applies to: 63-70, 73-86, 87-94, 97-104
74-85: LGTM: Structured class object.LanguageCodeTypeahead uses the new class object API for better styling control.
src/routes/(authenticated)/admin/settings/workflow-definitions/edit/+page.svelte (2)
52-57: LGTM: Deprecated class cleanup.Consistent removal of
select-borderedacross all select elements.Also applies to: 60-67, 79-84, 87-92
95-101: LGTM: Prop standardization.Component props consistently updated to use
classinstead ofclassName/classes.Also applies to: 103-114, 105-112
src/routes/(authenticated)/+layout.svelte (3)
5-5: LGTM: ClassValue integration for snippet.The snippet parameter update to
classes?: ClassValuewith array binding enables flexible class composition.Also applies to: 72-86
298-303: LGTM: Component API standardization.LocaleSelector and Dropdown now use structured class objects, providing better control over individual element styling.
Also applies to: 304-353
320-320: LGTM: Deprecated class update.Changed
menu-compacttomenu-smas noted in the PR description for deprecated class removal.src/lib/products/components/ReleaseInfo.svelte (1)
3-3: LGTM: Structured class object API.The migration to a nested class object with
defaultandheaderfields provides better organization for component styling.Also applies to: 7-24
src/lib/components/Dropdown.svelte (3)
46-47: LGTM!The array syntax for class binding with optional chaining is correct and handles the undefined case properly.
50-55: Dark mode styling enhancement looks good.The addition of
drop-shadow-lgand dark mode border (dark:border dark:border-base-content) aligns with the PR objectives to improve dropdown visibility. The array-based class binding correctly preserves custom classes from consumers.
8-25: API migration verified—all Dropdown consumers updated.All usages of the Dropdown component throughout the codebase have been successfully migrated to the new nested object syntax
class={{ dropdown, label, content }}. No old prop patterns remain.src/routes/(authenticated)/tasks/[product_id]/+page.svelte (3)
128-128: BlockIfJobsUnavailable component correctly accepts theclassprop.The component definition has been updated with the
class?: ClassValue;prop in the Props interface and properly destructures it asclasses. The usage in this file is correct and aligns with the project-wide standardization.
168-215: LabeledFormInput correctly accepts theclassprop.All six usages in lines 168–215 have been properly updated from
classestoclass. The component definition (LabeledFormInput.svelte line 14) acceptsclassand internally maps it toclassesfor use within the template.
314-416: SortTable successfully acceptsclassprop at both component and column levels.The component Props interface and column type definition have both been properly updated to accept
class?: ClassValue;. NoclassNamereferences remain in the component, and both the component-level usage (lines 314, 357, 416) and column-level usage (line 373) in the page are correctly aligned with the updated component signature.
1 participant
Fixes #1412
Add drop shadow:
Add border in dark mode (shadow wasn't particularly visible):
Fancy selector for Reviewer locale:
Dropped some deprecated classes discovered during process (select-bordered, menu-compact => menu-sm)
Updated class directives to newer syntax in several locations. Svelte recently updated docs to recommend against using
class:. This is still used in many places, but should be fine for now.I am open to splitting the class specific changes out into a separate PR if desired.
Summary by CodeRabbit
New Features
Bug Fixes
Style
✏️ Tip: You can customize this high-level summary in your review settings.