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

Add optional process func for selection text #57

Merged
merged 3 commits into from
Jun 4, 2024

Conversation

alondahari
Copy link
Member

@alondahari alondahari commented Jun 4, 2024

Adding this optional passed in processing function for the selection text before it is inserted back into the dom. This allows for doing things like search and replace if we need to.

This is done to address an issue we have where quoted assets are entered into the new comment with the short lived JWT. Since we already have the asset with the camo'd url on the client, this can be solved with a simple search and replace before we enter the quoted text into the dom.

Since this is an opt-in behaviour, it should not affect any existing consumers.

Adding this optional passed in processing function for the selection
text before it is inserted back into the dom. This allows for doing
things like search and replace if we need to.
@alondahari alondahari requested a review from a team as a code owner June 4, 2024 14:37
@mattcosta7
Copy link
Member

Adding this optional passed in processing function for the selection text before it is inserted back into the dom. This allows for doing things like search and replace if we need to.

This is done to address an issue we have where quoted assets are entered into the new comment with the short lived JWT. Since we already have the asset with the camo'd url on the client, this can be solved with a simple search and replace before we enter the quoted text into the dom.

Since this is an opt-in behaviour, it should not affect any existing consumers.

@alondahari can you add some tests for this new behavior?

@alondahari
Copy link
Member Author

Do we need to version bump as well?

@mattcosta7 mattcosta7 merged commit 91db2a9 into main Jun 4, 2024
3 checks passed
@mattcosta7 mattcosta7 deleted the allow-processing-selected-text branch June 4, 2024 17:38
@mattcosta7
Copy link
Member

Do we need to version bump as well?

i'm not sure - we seem to call version in the publish workflow, but may need to manually do that first? i'll check and we can update separately if necessary

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