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: make tag parsing work better #3038

Merged
merged 3 commits into from
May 14, 2020
Merged

Conversation

beyackle
Copy link
Contributor

Description

Trimming strings the way this currently works makes it awkward to enter tags with spaces in them; because the list is re-parsed and regenerated every keystroke, the spaces vanish as soon as they're input. Changing the split into a regex gives us the power to only care about spaces on either side of a comma and eliminate those, while leaving spaces inside tags just where they need to be. Note that this, cosmetically, requires no spaces around commas in the list (so "a,b,c" instead of "a, b, c") because otherwise it breaks the ability to backspace past a ", " unit - the backspace will delete the space, then the comma will regenerate it.

A real fix to this would involve changing when the form decides to re-parse and regenerate values (so we only do it when either swapping to/from JSON view or on hitting the Done button), but I don't know if we have that option.

Task Item

closes #3037

@github-actions
Copy link

Coverage Status

Coverage remained the same at 0.0% when pulling 88c56cb on beyackle/commaRegexInExport into 652868a on master.

@a-b-r-o-w-n a-b-r-o-w-n added the Approved to merge approved, waiting to be merged label May 14, 2020
@cwhitten cwhitten merged commit f83d78d into master May 14, 2020
@cwhitten cwhitten deleted the beyackle/commaRegexInExport branch May 14, 2020 15:56
lei9444 pushed a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved to merge approved, waiting to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants