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: remove markRaw #191

Merged
merged 1 commit into from
Mar 7, 2024
Merged

fix: remove markRaw #191

merged 1 commit into from
Mar 7, 2024

Conversation

juliodialpad
Copy link
Collaborator

Fix Rich Text editor - Remove markRaw

Obligatory GIF (super important!)

Obligatory GIF

๐Ÿ› ๏ธ Type Of Change

These types will increment the version number on release:

  • Fix

๐Ÿ“– Jira Ticket

No Jira ticket

๐Ÿ“– Description

Removed markRaw from vue2 version of DtRichTextEditor plugins as this is exclusive of vue 2.7+ and dialpad is using 2.6.14

๐Ÿ’ก Context

Issues implementing DtRecipeMessageInput on dialpad were reported, worked with @bianca-artola-dialpad and found out this markRaw issues.

๐Ÿ“ Checklist

For all PRs:

  • I have ensured no private Dialpad links or info are in the code or pull request description (Dialtone is a public repo!).
  • I have reviewed my changes.
  • I have considered the performance impact of my change.

๐Ÿ”ฎ Next Steps

Release and update on product side.

Copy link

github-actions bot commented Mar 6, 2024

โœ”๏ธ Deploy previews ready!
๐Ÿ˜Ž Dialtone preview: https://dialtone.dialpad.com/deploy-previews/pr-191/
๐Ÿ˜Ž Dialtone-vue 2 preview: https://dialtone.dialpad.com/vue/deploy-previews/pr-191/
๐Ÿ˜Ž Dialtone-vue 3 the preview: https://dialtone.dialpad.com/vue3/deploy-previews/pr-191/

@juliodialpad juliodialpad added the no-visual-test Add this tag when the PR does not need visual testing label Mar 6, 2024
Copy link
Contributor

@braddialpad braddialpad left a comment

Choose a reason for hiding this comment

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

What exactly are we losing by removing this?

@iropolo
Copy link
Contributor

iropolo commented Mar 7, 2024

Huh... why it was there in first place...? @sanjeevdialpad worked out that feature? maybe he has some info

@juliodialpad
Copy link
Collaborator Author

What exactly are we losing by removing this?

This might impact the performance, there is chance that the component could be made reactive and it doesn't need to, that's why it was trying to use markRaw in first place, tried to replace with Object.freeze() but it didn't work.

I didn't see the Vue warning related to component being reactive when I don't need to tho, so I think we're good.

@juliodialpad
Copy link
Collaborator Author

Huh... why it was there in first place...? @sanjeevdialpad worked out that feature? maybe he has some info

markRaw is to create static components that don't need reactivity but it doesn't work on vue 2.6.14

Copy link
Contributor

@braddialpad braddialpad left a comment

Choose a reason for hiding this comment

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

Alright let's give er a go

@juliodialpad juliodialpad merged commit 74884e2 into staging Mar 7, 2024
11 of 12 checks passed
@juliodialpad juliodialpad deleted the fix/rich-text-editor branch March 7, 2024 18:36
braddialpad pushed a commit that referenced this pull request Mar 7, 2024
# [2.119.0](dialtone-vue2/v2.118.0...dialtone-vue2/v2.119.0) (2024-03-07)

### Bug Fixes

* message input issues ([#190](#190)) ([caa07b6](caa07b6))
* remove markRaw ([#191](#191)) ([74884e2](74884e2))

### Features

* **Editor:** add quick replies icon to editor ([#193](#193)) ([7a6829f](7a6829f))
braddialpad pushed a commit that referenced this pull request Mar 7, 2024
# [9.18.0](dialtone/v9.17.0...dialtone/v9.18.0) (2024-03-07)

### Bug Fixes

* message input issues ([#190](#190)) ([caa07b6](caa07b6))
* remove markRaw ([#191](#191)) ([74884e2](74884e2))

### Documentation

* use CodeExampleTabs from badge to button-group ([#185](#185)) ([808015b](808015b))

### Features

* **Editor:** add quick replies icon to editor ([#193](#193)) ([7a6829f](7a6829f))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-visual-test Add this tag when the PR does not need visual testing
Projects
None yet
3 participants