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(emoji-picker): NO-JIRA refactor fix ref reactivity in vue2 #3

Open
wants to merge 1 commit into
base: staging
Choose a base branch
from

Conversation

lizard-boy
Copy link

Refactor fix ref reactivity in vue2

This is a refactor of this fix dialpad#512
The previous fix does not work as expected in product DP-113081, probably because the vue version (2.6.14).
This is not something new, I deal previously with this kind of issues.

This PR is just a refactor of the same fix, Im just doing it in a different way and this one seems to work.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR refactors the emoji picker component in Vue 2 to address reactivity issues encountered in the product environment, specifically related to DP-113081.

  • Introduced new tabLabelsRefs data property to store references to tab labels
  • Updated setupTabLabelRefs method to use tabLabelsRefs for improved reactivity
  • Modified scrollToTab method to access tab elements using tabLabelsRefs
  • Changes aim to resolve reactivity problems in Vue 2.6.14 environment

1 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings

Comment on lines 258 to 262
this.tabSetLabels?.forEach((_, index) => {
const refKey = `tabLabelRef-${index}`;
if (this.$refs[refKey]) {
this.$set(this.tabLabels, index, { ...this.tabLabels[index], ref: this.$refs[refKey] });
this.$set(this.tabLabelsRefs, index, { ref: this.$refs[refKey] });
}
Copy link

Choose a reason for hiding this comment

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

logic: This approach may still have reactivity issues. Consider using Vue.set() or this.$set() to ensure reactivity

@@ -329,8 +330,7 @@ export default {
scrollToTab: function (tabIndex, focusFirstEmoji) {
const vm = this;
if (focusFirstEmoji === undefined) { focusFirstEmoji = true; }
const tabLabel = vm.tabLabels[tabIndex - 1];
const tabElement = tabLabel.ref[0];
const tabElement = vm.tabLabelsRefs[tabIndex - 1].ref[0];
Copy link

Choose a reason for hiding this comment

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

logic: Ensure tabIndex - 1 is always >= 0 to prevent accessing undefined array elements

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.

1 participant