Skip to content

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
@alondahari alondahari requested a review from mattcosta7 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
@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.

5 participants