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

feat: Allow multiple prefix characters to trigger a suggestion #2896

Merged
merged 2 commits into from Jun 21, 2022

Conversation

rfgamaral
Copy link
Contributor

This PR replaces the prefixSpace option in the Suggestion utility with a new, more flexible option, allowedPrefixes. This option accepts multiple characters in a string, and allows each one of those characters as a prefix when triggering suggestions.

The current prefixSpace option is not very flexible, only allowing spaces as the prefix character, or everything else. But we are in the need of something more flexible, where we only allow a certain set of characters to be prefixed before the suggestion trigger character (i.e. @ for the mention extension).

The idea here is that we can configure this option to work more like GitHub. For instance, setting allowedPrefixes: ' ([', suggestions can be triggered if the prefix character is , ( or [. The default is , which works exactly like the current default. Setting allowedPrefixes: null, disables the option, and any character is allowed to be a prefix (same as setting prefixSpace: false in the current implementation).

I've also added allowedPrefixes to the Suggestion utility documentation, whereas prefixSpace was currently missing.

@netlify
Copy link

netlify bot commented Jun 17, 2022

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit 9d7a8d7
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/62b189573d1c6300080d613b
😎 Deploy Preview https://deploy-preview-2896--tiptap-embed.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@rfgamaral rfgamaral force-pushed the multiple-suggestion-prefixes branch 5 times, most recently from ec4f28f to 74f9588 Compare June 20, 2022 16:42
@rfgamaral
Copy link
Contributor Author

@bdbch Any clue why Netlify deployment is failing?

@bdbch
Copy link
Contributor

bdbch commented Jun 20, 2022

@rfgamaral we had the same issue. Seems like Netlify cached something and now doesn't like new builds. I'll hit up a fresh run for this PR! :)

@bdbch
Copy link
Contributor

bdbch commented Jun 20, 2022

I manually triggered a build:
https://app.netlify.com/sites/tiptap-embed/deploys/62b0afc310168b6a4ba03e5b

The PR looks fine to me.

@bdbch bdbch self-requested a review June 20, 2022 17:38
Copy link
Contributor

@bdbch bdbch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a quick comment about the documentation / clarity of the explanation.

Otherwise this LGTM

@rfgamaral rfgamaral force-pushed the multiple-suggestion-prefixes branch from 74f9588 to 9d7a8d7 Compare June 21, 2022 09:03
@bdbch bdbch merged commit 482cb96 into ueberdosis:main Jun 21, 2022
@rfgamaral rfgamaral deleted the multiple-suggestion-prefixes branch June 22, 2022 16:01
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.

None yet

2 participants