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.

Sign up for GitHub

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

Jump to bottom

fix: remove setup() and move code to data() in Results.vue #2065

Merged
merged 1 commit into from
Apr 15, 2024
Merged

fix: remove setup() and move code to data() in Results.vue #2065

merged 1 commit into from
Apr 15, 2024

Conversation

Chartman123
Copy link
Collaborator

Signed-off-by: Christian Hartmann chris-hartmann@gmx.de

@Chartman123
fix: remove setup() and move code to data() in Results.vue
56f410a 
Signed-off-by: Christian Hartmann <chris-hartmann@gmx.de>
@Chartman123 Chartman123 added the 3. to review Waiting for reviews label Apr 15, 2024
@Chartman123 Chartman123 requested a review from susnux April 15, 2024 07:29
@Chartman123 Chartman123 self-assigned this Apr 15, 2024
@Chartman123 Chartman123 added regression Regression of a previous working feature feature: 📊 responses & statistics labels Apr 15, 2024
@Chartman123 Chartman123 added this to the 4.2.1 milestone Apr 15, 2024
@Chartman123 Chartman123 linked an issue Apr 15, 2024 that may be closed by this pull request
"Linked file not available" panel buttons do not work on a form in 4.2.0 #2063
Closed
susnux
susnux requested changes Apr 15, 2024
View reviewed changes
Copy link
Collaborator

@susnux susnux left a comment

Choose a reason for hiding this comment

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

Yes and no. Everything reactive should be in data (except we switch to Composition API), but the composables should be and must be in setup

@Chartman123
Copy link
Collaborator Author

Ok, could you please fix my code then?

@susnux susnux merged commit 56f410a into main Apr 15, 2024
43 checks passed
@susnux susnux deleted the fix/unlink-file branch April 15, 2024 14:54
@susnux
Copy link
Collaborator

susnux commented Apr 15, 2024

@susnux susnux merged commit 56f410a into main Apr 15, 2024

I am pretty sure I merged the other PR not this one.

@Chartman123
Copy link
Collaborator Author

Ok, should we revert it partially? I just read that setup() is only supported with vue3, that's why I moved everything into data().

@susnux
Copy link
Collaborator

susnux commented Apr 15, 2024

Ok, should we revert it partially? I just read that setup() is only supported with vue3

No, setup is supported since Vue 2.7
Everything in data will be made reactive, things in setup not. In setup you use the Composition API to make things reactive (e.g. const foo = ref(4)).

But some things like composables need to be defined in setup to allow them to register all life-cycle hooks like mounted.

@Chartman123 Chartman123 mentioned this pull request Sep 19, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@susnux susnux susnux requested changes

Assignees

@Chartman123 Chartman123

Labels
3. to review Waiting for reviews feature: 📊 responses & statistics regression Regression of a previous working feature
Projects
None yet
Milestone
4.2.1
Development

Successfully merging this pull request may close these issues.

"Linked file not available" panel buttons do not work on a form in 4.2.0
2 participants
@Chartman123 @susnux