search

unovue / radix-vue

Vue port of Radix UI Primitives. An open-source UI component library for building high-quality, accessible design systems and web apps.
https://radix-vue.com
MIT License
3.7k stars 229 forks source link

feat(TagsInput): support multiple delimiters via RegExp #1408

Closed romansp closed 2 weeks ago

romansp commented 3 weeks ago

Resolves #1400.

Added support for RegExp delimiter prop in TagsInput.

Open questions:

  1. Should we do our own validation of passed RegExp to ensure global flag is set. Without global flag String.prototype.replaceAll will produce the following error: Uncaught TypeError: String.prototype.replaceAll called with a non-global RegExp argument
  2. Consider support for string[] delimiter. If we do then decide:
    1. Convert string[] into RegExp. Seems somewhat tricky to do it correctly and account for all the edge cases, e.g. escaping special characters.
    2. Iterate delimiters and replace/split for each delimiter. Quite straightforward but will require slightly bigger rework in handleInput and handlePaste.

Do you have any thoughts on these @zernonia? Or maybe other suggestions?

zernonia commented 3 weeks ago

We also need to apply the changes to handlePaste

romansp commented 3 weeks ago

@zernonia thanks for the review.

We also need to apply the changes to handlePaste

I don't think any additional changes are needed for handlePaste. String.prototype.split accepts RegExp argument. I tested it with /[ ,;]+/ and it worked as expected on paste. But maybe you meant something else?

Would you like me to add an example to docs showcasing multiple delimiters usage?

romansp commented 3 weeks ago

PR updated:

  1. switch to replace instead of replaceAll to not require global flag on regexp
  2. updated delimiter prop description
  3. added example for multiple delimiters usage with regexp
zernonia commented 2 weeks ago

Nicely done @romansp !! Working well! 🚀 We have freeze any new feature to main branch, perhaps we can push this to v2? 😁

Also, do you mind quickly adding in some test case for this?

romansp commented 2 weeks ago

@zernonia Thanks! I'm good with this being pushed to v2. Do you want me to change target branch or you do it?

Let me check what type of test case we can add here. We could probably verify that typing text with delimiters creates tags correctly? I will try to add coverage for paste event as well.

zernonia commented 2 weeks ago

Super welcome @romansp !! Once again thanks for the PR! Sorry I messed up the branch, I've cherry-picked your commit to https://github.com/unovue/radix-vue/pull/1414 😁