Feat/add disabled to listbox listboxitem #2485
Conversation
… ListBoxItem components. Added documentation for it
🦋 Changeset detectedLatest commit: b930cc7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -17,6 +19,8 @@ | |||
| // Props (styles) - Item Only | |||
| /** Provide classes to set the listbox item active background styles. */ | |||
| export let active: CssClasses = 'variant-filled'; | |||
| /** Provide classes to set the listbox item active disabled background styles */ | |||
| export let activeDisabled: CssClasses = 'variant-ghost'; | |||
There was a problem hiding this comment.
Typically, we don't offer a "disabled-style" prop.
Instead, we provide a default styling for disabled elements (* users can always override these styles). However, if we decide to introduce such a feature, let's consider renaming the prop and updating the corresponding comment for clarity.
There was a problem hiding this comment.
I've renamed the class and holding on removing if there is an intention in providing this feature, as you mentioned.
There was a problem hiding this comment.
This can be closed if you agree with #2485 (comment)
packages/skeleton/src/lib/components/ListBox/ListBoxItem.svelte
Outdated
Show resolved
Hide resolved
| export let hover: CssClasses = getContext('hover'); | ||
| export let padding: CssClasses = getContext('padding'); | ||
| export let regionLead: CssClasses = getContext('regionLead'); | ||
| export let regionDefault: CssClasses = getContext('regionDefault'); | ||
| export let regionTrail: CssClasses = getContext('regionTrail'); | ||
|
|
||
| // Classes | ||
| const cBase = 'cursor-pointer -outline-offset-[3px]'; | ||
| const cBase = '-outline-offset-[3px]'; |
There was a problem hiding this comment.
let's re-add cursor-pointer here and override it using the disabled style instead.
There was a problem hiding this comment.
I've re added it but I'm not sure how can i override the disabled style. Could you elaborate more on that?
There was a problem hiding this comment.
look at https://github.com/skeletonlabs/skeleton/blob/c6a4ddc02d2883e2ff0c350cc5f02f3b69d0d9ce/packages/skeleton/src/lib/components/TreeView/TreeViewItem.svelte#L320C33-L320C51
for how we handled this in other components 👍
There was a problem hiding this comment.
Ok, let me know if i'm doing this correctly. I've removed entirely the possibility for styling the active disabled items and replaced it with opacity which seems better, using the cDisabled approach.
sites/skeleton.dev/src/routes/(inner)/components/listboxes/+page.svelte
Outdated
Show resolved
Hide resolved
…sabled approach.
|
@fpribeiro3069 the PR was great, just a couple fixes needed:
I've made the changes for you and this satisfies all requirements on our end, so I'll merge and this will be part of tomorrow's release. Thanks for your contribution! |
Linked Issue
Closes #2484
Description
Added disabled prop to ListBox and ListBoxItem to prevent selection of inner radio/checkbox inputs.
A ListBoxItem disabled will:
Changsets
Instructions: Changesets automate our changelog. If you modify files in
/packages, runpnpm changesetin the root of the monorepo, follow the prompts, then commit the markdown file. Changes that add features should beminorwhile chores and bugfixes should bepatch. Please prefix the changeset message withfeat:,bugfix:orchore:.Checklist
Please read and apply all contribution requirements.
devbranch (NEVERmaster)docs/,feat/,chore/,bugfix/pnpm ci:checkpnpm formatpnpm test