Skip to content

Space being unavoidably added #35

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

Closed
silverwind opened this issue Apr 7, 2023 · 5 comments · Fixed by #36
Closed

Space being unavoidably added #35

silverwind opened this issue Apr 7, 2023 · 5 comments · Fixed by #36

Comments

@silverwind
Copy link
Contributor

silverwind commented Apr 7, 2023

There is an unavoidable space character being appended target element's value during the text-expander-value event:

const value = `${detail.value} `

Could this be removed or made optional? I think the consumer of the module should be in control of whether they want this space, or not.

@keithamus
Copy link
Contributor

Thanks for the issue! PRs welcome to add the optionality.

@silverwind
Copy link
Contributor Author

silverwind commented Apr 8, 2023

How do you feel about removing it and pushing v3.0.0 with it? I think the proper fix is to remove it instead of working around the issue with an option.

@keithamus
Copy link
Contributor

We’d like to retain the behaviour so if you think it’s undesirable for your usecase, then we can make it an option (eg a suffix attribute which defaults to a space)

@silverwind
Copy link
Contributor Author

silverwind commented Apr 8, 2023

My use case might be something for GitHub to consider too:

  • On a @ mention, the space is desirable as one usually starts a sentence.
  • On a : emoji, the space is undesirable as it's often used to end a sentence.

A HTML attribute will not work as I need different behaviour per-key, so it needs to be an option that is set before event.detail.value is assigned. I'm thinking about event.detail.options as a options object with default value {suffix: ' '}.

@silverwind
Copy link
Contributor Author

silverwind commented Apr 8, 2023

Actually, a HTML option will work as well when set to the empty string as the optional space can be appended during the event.detail.value assignment based on key, so I'll go for that.

silverwind added a commit to silverwind/text-expander-element that referenced this issue Apr 8, 2023
silverwind added a commit to silverwind/text-expander-element that referenced this issue Apr 8, 2023
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 a pull request may close this issue.

2 participants