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

frontend: add simple part of vue/vue3-recommended rules #5170

Merged
merged 1 commit into from
Jun 2, 2024

Conversation

BacLuc
Copy link
Contributor

@BacLuc BacLuc commented May 12, 2024

Turn off the following rules for now:
"vue/no-deprecated-dollar-scopedslots-api": "off",
"vue/no-deprecated-slot-attribute": "off",
"vue/no-deprecated-slot-scope-attribute": "off",
because i could not get the new syntax to work.

That we are ready for vue3 as soon as the calendar works. Issue: #5121

@BacLuc BacLuc added the deploy! Creates a feature branch deployment for this PR label May 12, 2024
Copy link

github-actions bot commented May 12, 2024

Feature branch deployment currently inactive.

If the PR is still open, you can add the deploy! label to this PR to trigger a feature branch deployment.

BacLuc added a commit to BacLuc/ecamp3 that referenced this pull request May 12, 2024
That we have tests when we turn the following rules on:
"vue/no-deprecated-dollar-scopedslots-api": "off",
"vue/no-deprecated-slot-attribute": "off",
"vue/no-deprecated-slot-scope-attribute": "off",

Issue: ecamp#5121

Related to: ecamp#5170, ecamp#5122
Copy link
Member

@pmattmann pmattmann left a comment

Choose a reason for hiding this comment

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

I'm unsure if it has to do with this PR, but I can't create new activities on the Picasso (by dragging).

@BacLuc
Copy link
Contributor Author

BacLuc commented May 19, 2024

Seems that the lint rule rewrites the event identifier where you register the event, but not the place where you emit the event.
Fixed with fc71576

@BacLuc BacLuc requested a review from pmattmann May 19, 2024 18:08
@@ -7,8 +7,8 @@
class="drag-and-drop-handle"
:disabled="isLastSection"
:aria-label="$tc('global.button.move')"
@keydown.down="$emit('moveDown', itemKey)"
@keydown.up="$emit('moveUp', itemKey)"
@keydown.down="$emit('move-down', itemKey)"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@manuelmeister
Should it be possible to move the storyboard entries here around with the keyboard?
how do i test this?

@@ -42,7 +42,7 @@ export default {
}
}
},
destroyed() {
unmounted() {
Copy link
Member

Choose a reason for hiding this comment

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

According to vue2 docs, there are no lifecycle hooks unmounted or beforeUnmount. Did you check if this code is still called?

https://v2.vuejs.org/v2/guide/instance#Lifecycle-Diagram
https://v2.vuejs.org/v2/api/#Options-Lifecycle-Hooks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i reverted that change.
They were not called

@@ -20,6 +20,7 @@ Displays a field as a date picker (can be used with v-model)
>
<template #default="picker">
<v-date-picker
:picker-date="pickerMonth"
Copy link
Member

Choose a reason for hiding this comment

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

Why is the .sync modifier not needed anymore?
(same question for the other places, where the .sync modifier was removed)

Copy link
Member

Choose a reason for hiding this comment

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

In vue3 there's a replacement for multiple 2-way-binding properties (argument for v-model):
https://v3-migration.vuejs.org/breaking-changes/v-model

But I guess this is not working in vue2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I remember vaguely that i had some problems with the .sync modifier.
But i could not reproduce it, so i added the .sync modifier, but changed the attribute order.

@BacLuc BacLuc requested a review from usu May 26, 2024 12:26
Copy link
Member

@usu usu left a comment

Choose a reason for hiding this comment

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

Other than the comment below, looks good to me 👍

frontend/src/components/program/picasso/PicassoEntry.vue Outdated Show resolved Hide resolved
@BacLuc BacLuc requested a review from usu June 2, 2024 11:31
@usu usu enabled auto-merge June 2, 2024 11:35
@usu usu added this pull request to the merge queue Jun 2, 2024
Turn off the following rules for now:
"vue/no-deprecated-dollar-scopedslots-api": "off",
"vue/no-deprecated-slot-attribute": "off",
"vue/no-deprecated-slot-scope-attribute": "off",
because i could not get the new syntax to work.

That we are ready for vue3 as soon as the calendar works.
Issue: ecamp#5121
@BacLuc BacLuc force-pushed the frontend-add-part-of-vue3-recommended branch from 7b2788c to 8507f39 Compare June 2, 2024 11:47
@BacLuc
Copy link
Contributor Author

BacLuc commented Jun 2, 2024

More rule applications were here: #5122

Merged via the queue into ecamp:devel with commit 2164df1 Jun 2, 2024
24 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy! Creates a feature branch deployment for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants