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

Changes for Vue Upgrade #108

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

HeriLFIU
Copy link
Collaborator

Closes #107
Closes #106

Partially deals with #85 but the toolbar still uses Vue instead of app instance to register components with the httpvueloader.

Summary

I updated the input v-model to use the new syntax.
I replaced $listerners with $attrs in the auto complete component.
I replaced the Vue and new Vue with the app instance in main.js.

Local Tests

The warnings were removed from the console.
The UI still functioned as should.

@HeriLFIU HeriLFIU requested a review from a team as a code owner October 14, 2024 15:42
@@ -21,11 +21,14 @@ import KytosBaseWithIcon from '../base/KytosBaseWithIcon';
export default {
name: 'k-input',
mixins: [KytosBaseWithIcon],
compatConfig: {
MODE: 3,

Choose a reason for hiding this comment

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

very nice that you changed the compatibility mode to 3, @HeriLFIU !

Choose a reason for hiding this comment

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

can we change it globally ? is there any broken code that would be impacted ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you, but I only changed it for two individual components, since the changes worked in vue3 but did not work in vue2, since they were incompatible.

Choose a reason for hiding this comment

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

I've changed it globally to MODE3 on webpack/webpack.common.js just for testing and it seems to work fine (despite additional warnings on compilation)

@HeriLFIU
Copy link
Collaborator Author

@rmotitsuki do you think it would be a good idea to globally enable MODE3 on the vue js compat?

@italovalcy
Copy link

@HeriLFIU I tested here your pull request (without enabling MODE3 globally, i.e., strictly using the changes proposed here) and I ended up observing a weird behavior: If I try to create and EVC, filled out the Circuit Name, then press TAB to choose endpoint A, then the circuit name disappear.

See this demo after applying your changes:

Oct-14-2024 15-59-39

The procedure I'm using to build and apply the changes is:

git clone https://github.com/kytos-ng/ui
cd ui
curl -LO https://github.com/kytos-ng/ui/pull/108.diff
patch -p1 < 108.diff
docker run -d --name vue3 -v $(PWD):/kytos-ui -w /kytos-ui node:lts-alpine tail -f /dev/null
docker exec -it vue3 sh
apk add zip
rm -rf latest.zip dist package-lock.json node_modules/
npm install && npm run preprod && zip -r latest.zip index.html dist
exit
mv /usr/local/lib/python3.11/dist-packages/kytos/web-ui /usr/local/lib/python3.11/dist-packages/kytos/web-ui-$(date +%Y%m%d%H%M)
python3 -m zipfile -e latest.zip /usr/local/lib/python3.11/dist-packages/kytos/web-ui

@HeriLFIU
Copy link
Collaborator Author

That's strange, that's most likely due to replacing $listeners with $attrs in the k-input-auto. I thought it should have acted similarly to the $listeners code, but ill look into it.

@HeriLFIU
Copy link
Collaborator Author

@italovalcy thank you for the help, I tested it out and it should be working now. The reason for the issue was that a fix for the v-model was already implemented, after reading the documentation you could have had a default value/variable for v-model. In vue js 2 this default value would be defined with just value, but now its defined with modelValue. Or you could specify a value to track by name using v-model:name="". Instead of using the default value for v-model the code already set that it was going to track value by stating v-model:value="". So there was no need for the v-model change.

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.

v-model no longer uses a default value prop and a default input event $listeners object is no longer in use
2 participants