Skip to content

feat: Add data attributes for selected and active items in MulitSelect#1877

Merged
shinokada merged 3 commits intothemesberg:mainfrom
PhilipHansen:patch-1
Dec 11, 2025
Merged

feat: Add data attributes for selected and active items in MulitSelect#1877
shinokada merged 3 commits intothemesberg:mainfrom
PhilipHansen:patch-1

Conversation

@PhilipHansen
Copy link
Contributor

@PhilipHansen PhilipHansen commented Dec 10, 2025

📑 Description

Adds data-selected and data-active attributes to items in the dropdown within the MultiSelect component


🔍 PR Type

  • Bug fix
  • Feature
  • Documentation
  • Refactor / Code cleanup
  • Build / Tooling
  • Other (please describe)

🚦 PR Status

  • Draft (work in progress, not ready for review)
  • Ready for review ✅

✅ Checklist

  • My code follows the existing code style
  • I have run pnpm lint && pnpm check && pnpm test:e2e and all tests pass
  • CoderabbitAI review has been completed and actionable suggestions were reviewed
  • I have updated documentation if my changes require it
  • My PR is based on the latest main branch (not the published npm version)
  • I have checked accessibility where applicable (ARIA, keyboard nav, etc.)
  • I have reviewed the rendered component in the browser

ℹ️ Additional Information

While using Flowbite within our Svelte project, we have been trying to find a way to customize the styling of the items within the dropdown of the MultiSelect component. We can add classes to the items, but couldn't find a good way to style them when selected or active. We found that by adding these data- attributes to the items, we were able to solve this problem, in the least intrusive manner possible.

Summary by CodeRabbit

  • New Features
    • MultiSelect dropdown items now expose per-item selection and active state via element attributes and class bindings, enabling finer styling and interaction customization without changing external behavior.
    • These attributes provide explicit hooks for styling and accessibility without altering existing functionality.

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel
Copy link

vercel bot commented Dec 10, 2025

@PhilipHansen is attempting to deploy a commit to the Themesberg Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 10, 2025

Walkthrough

Adds per-item bindings isSelected and isActive in the MultiSelect item loop and exposes those states via data-selected and data-active attributes on each dropdown item div; replaces inline selected/active computations with the new bindings.

Changes

Cohort / File(s) Summary
Data & state attributes in MultiSelect
src/lib/forms/select/MultiSelect.svelte
Added per-item bindings isSelected and isActive; replaced inline selected/active computations with those bindings; added data-selected and data-active attributes and mapped class bindings (selected, active) to the new bindings.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~7 minutes

  • Check the each-block bindings and any key usage for correctness.
  • Verify class bindings and new data-* attributes do not break styling or accessibility.

Possibly related PRs

Poem

🐰 I nibble through options, tidy and neat,
I mark each chosen hop with a beat,
data-selected and data-active I sing,
Little states shining on every spring,
Hop on — the dropdown's a meadow to meet.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding data attributes for selected and active items in the MultiSelect component, which aligns with the actual changes made.
Description check ✅ Passed The PR description covers the key sections including purpose, PR type selection, status, and checklist completion. However, documentation updates remain unchecked, and the related issue section is missing.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 63f701b and 26d74ae.

📒 Files selected for processing (1)
  • src/lib/forms/select/MultiSelect.svelte (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/forms/select/MultiSelect.svelte

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/lib/forms/select/MultiSelect.svelte (1)

248-260: Optional: make data attributes presence-based and avoid duplicate expressions

This works as-is, but two small refinements could make it nicer for consumers:

  • data-selected / data-active will always be present with "true" or "false". For styling and tests it’s often more ergonomic if the attribute only exists when the state is true (so you can use [data-selected] / [data-active]).
  • The selectItems.includes(item) and activeItem === item checks are now repeated three times in this block.

You could refactor like:

{#each items as item (item.value)}
  {#key item.value}
    {#let isSelected = selectItems.includes(item)}
    {#let isActive = activeItem === item}
    <div
      onclick={(e) => selectOption(item, e)}
      role="presentation"
      class={dropdownItem({
        selected: isSelected,
        active: isActive,
        disabled: item.disabled,
        class: clsx(classes?.item)
      })}
      data-selected={isSelected ? "true" : undefined}
      data-active={isActive ? "true" : undefined}
    >
      {item.name}
    </div>
  {/key}
{/each}

(or similar using local const variables inside the loop), keeping behavior but simplifying usage and avoiding repeated checks.

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8e360bf and 35b3e72.

📒 Files selected for processing (1)
  • src/lib/forms/select/MultiSelect.svelte (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{svelte,ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{svelte,ts,tsx,js,jsx}: Use the Flowbite-Svelte MCP server to discover components by name or category before implementing UI components
Always use findComponent tool FIRST to locate the correct Flowbite-Svelte component before fetching documentation
After calling findComponent, use getComponentDoc tool to fetch complete documentation including usage examples, props, and best practices for Flowbite-Svelte components

Files:

  • src/lib/forms/select/MultiSelect.svelte

@shinokada
Copy link
Collaborator

Please see Coderabbitai's Nitpick comment.

@PhilipHansen
Copy link
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 10, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@shinokada
Copy link
Collaborator

Can you run pnpm format && pnpm lint && pnpm check && pnpm test:e2e to check if all pass?

@PhilipHansen
Copy link
Contributor Author

@shinokada Apologies -- did that the first time, but not after applying CodeRabbit's suggestion

@shinokada shinokada merged commit cacc822 into themesberg:main Dec 11, 2025
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants