Skip to content

Conversation

@ak4zh
Copy link
Contributor

@ak4zh ak4zh commented Aug 22, 2023

Description

InputChips was not updating when the selection changes.

Checklist

Please read and apply all contribution requirements.

  • This PR targets the dev branch (NEVER master)
  • Documentation reflects all relevant changes
  • Branch is prefixed with: docs/, feat/, chore/, bugfix/
  • Ensure Svelte and Typescript linting is current - run pnpm check
  • Ensure Prettier linting is current - run pnpm format
  • All test cases are passing - run pnpm test
  • Includes a changeset (if relevant; see above)

@changeset-bot
Copy link

changeset-bot bot commented Aug 22, 2023

🦋 Changeset detected

Latest commit: a6a0f1c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@skeletonlabs/skeleton Patch

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

@vercel
Copy link

vercel bot commented Aug 22, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
skeleton-docs ✅ Ready (Inspect) Visit Preview Aug 22, 2023 0:08am

@ak4zh ak4zh marked this pull request as ready for review August 22, 2023 00:10
@Sarenor
Copy link
Contributor

Sarenor commented Aug 22, 2023

Fixed #1922 now that #1922 exists

@endigo9740
Copy link
Contributor

endigo9740 commented Aug 22, 2023

@ak4zh two things here:

  1. Make sure not to modify the PR template. We've implemented all fields that we wish to be populated. This is why @Sarenor had to post the linked issues separately.
  2. I'm not sure I agree with the change proposed here. While it may solve the issue at hand, changing a huge swath of code to be reactive is like using a jackhammer to hammer in a nail - it's overkill. The more code the you make reactive, the more performance is impacted. I don't think we can move forward with this approach.

I'll try to sit down and review this issue in depth this week, as we've had a lot of comments regarding the Autocomplete not working as intended in certain scenarios. I'd like to try and find the root of the issue and review more optimal ways to solve for this. So I appreciate the PR, but for now this will be on pause until I can revisit.

@endigo9740 endigo9740 closed this Aug 22, 2023
@LarsHadidi
Copy link

Thank you 😃

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.

4 participants