Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prevent duplicate tag addition #2540

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kabilansakthivelu
Copy link
Contributor

@kabilansakthivelu kabilansakthivelu commented Feb 13, 2025

Description

  • In TaggedInput, added a check to avoid duplicate tag addition
  • Since in uncontrolled tagged input component, where the onChange will be called from useControllableState hook, wrapped the onTagChange for controlled component inside else branch to avoid getting called twice for uncontrolled component

Changes

Current behaviour:

  • In TaggedInput, the key prop is constructed for each tag using key={${tagName}-${tagIndex}}
  • In case if duplicate tags are added, then the key for each tag gets reset on removal which is causing the other duplicated tag (which gets previously deleted tag index) non visible as it gets isTagVisible prop value as false and few other duplicated tags as non-removable
Screen.Recording.2025-02-13.at.16.47.21.mov

Changes made:

  • Added a check to avoid duplicate tag entry
  • Wrapped the onTagChange invocation part inside else branch to avoid getting called twice incase of uncontrolled tagged input as it will be called once through useControllableState hook
Screen.Recording.2025-02-13.at.16.51.52.mov

Kindly suggest whether we need to prevent the duplicate tag addition or do we need to remove the already existing tag of same input value on hitting [Enter]

Additional Information

Component Checklist

  • Update Component Status Page
  • Perform Manual Testing in Other Browsers
  • Add KitchenSink Story
  • Add Interaction Tests (if applicable)
  • Add changeset

Copy link

changeset-bot bot commented Feb 13, 2025

⚠️ No Changeset found

Latest commit: 905d2f9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

github-actions bot commented Feb 13, 2025

✅ PR title follows Conventional Commits specification.

Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 905d2f9:

Sandbox Source
razorpay/blade: basic Configuration

@rzpcibot
Copy link
Collaborator

Bundle Size Report

Updated Components
Status Component Base Size (kb) Current Size (kb) Diff
TextArea 32.722 32.724 +0.002 KB
TextInput 34.262 34.263 +0.001 KB

Generated by 🚫 dangerJS against 905d2f9

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