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

chore: define explicit reactivity for webrtc models #11993

Merged
merged 2 commits into from
Apr 4, 2024

Conversation

DorraJaouad
Copy link
Contributor

@DorraJaouad DorraJaouad commented Apr 3, 2024

☑️ Resolves

Components affected

  • CallView
  • TopBar
  • TopBarMediaControls

🏁 Checklist

  • 🌏 Tested with Chrome, Firefox and Safari or should not be risky to browser differences
  • 🖥️ Tested with Desktop client or should not be risky for it
  • 🖌️ Design was reviewed, approved or inspired by the design team
  • ⛑️ Tests are included or not possible
  • 📗 User documentation in https://github.com/nextcloud/documentation/tree/master/user_manual/talk has been updated or is not required

Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
@DorraJaouad DorraJaouad force-pushed the fix/9450/explicit-reactivity branch from adb6443 to 853249c Compare April 4, 2024 10:47
@DorraJaouad DorraJaouad marked this pull request as ready for review April 4, 2024 10:48
@DorraJaouad DorraJaouad requested a review from ShGKme April 4, 2024 10:48
@ShGKme ShGKme requested a review from danxuliu April 4, 2024 14:15
Comment on lines +34 to +35
// FIXME: use reactive instead of ref after migration to vue 3
this.callParticipantModels = ref([])
Copy link
Contributor

Choose a reason for hiding this comment

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

[Vue warn]: Avoid using Array as root value for reactive() as it cannot be tracked in watch() or watchEffect(). Use ref() instead. This is a Vue-2-only limitation.

As we don't use it in watch or watchEffect, reactive would also work here.

Though, it will show the error in the console...
So, I don't know what is the best way here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, reactive works fine but those errors were annoying, I would keep ref temporarily.

@DorraJaouad DorraJaouad mentioned this pull request Apr 4, 2024
47 tasks
Copy link
Contributor

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

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

Thanks for separating into commits ☺️

@DorraJaouad DorraJaouad merged commit 5b870fc into main Apr 4, 2024
48 checks passed
@DorraJaouad DorraJaouad deleted the fix/9450/explicit-reactivity branch April 4, 2024 20:54
@Antreesy
Copy link
Contributor

/backport to stable29

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Always use reactive for external object that made reactive by putting in components data
3 participants