feat: sort theme and dropdown options case-insensitively#23
feat: sort theme and dropdown options case-insensitively#23adamalston wants to merge 4 commits intozerebos:mainfrom
Conversation
src/lib/stores/config.svelte.ts
Outdated
| const {type, id, value} = setting; | ||
|
|
||
| if ((type === "dropdown" || type === "theme") && Array.isArray(setting.options)) { | ||
| setting.options = [...setting.options].sort((a, b) => { |
There was a problem hiding this comment.
Why recreate the array rather than sort in place?
There was a problem hiding this comment.
Pull request overview
This PR adds case-insensitive alphabetical sorting to dropdown and theme options to improve the user experience when selecting values from these lists.
Changes:
- Added sorting logic for dropdown and theme options that handles both string options and object options (with name/value pairs) case-insensitively
- Refactored the initialization loop to use destructured variables for better readability
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/lib/stores/config.svelte.ts
Outdated
| if ((type === "dropdown" || type === "theme") && Array.isArray(setting.options)) { | ||
| setting.options.sort((a, b) => { | ||
| const normalizedA = (typeof a === "string" ? a : a.name).toLowerCase(); | ||
| const normalizedB = (typeof b === "string" ? b : b.name).toLowerCase(); | ||
|
|
||
| return normalizedA.localeCompare(normalizedB); | ||
| }); | ||
| } |
There was a problem hiding this comment.
The sorting is performed during module initialization, but theme options are populated asynchronously via fetchThemeFiles() in settings.ts (line 119-124). This means theme files fetched from the GitHub API will be added to the options array after this sort runs, leaving them unsorted. Consider either:
- Moving the sort logic to run after theme options are loaded (e.g., in the fetchThemeFiles().then() callback)
- Implementing reactive sorting that triggers whenever options are modified
- Sorting the theme names before pushing them in the fetchThemeFiles callback
|
I'm assuming you've tested this and the asynchronous issue copilot is referencing is not relevant? |
|
What Copilot is mentioning is likely a better solution. |
2 participants
Closes #22