Replace httpvueloader with vue3-sfc-loader #109
Conversation
HeriLFIU
changed the title
|
@viniarck I just finished testing all of the k-toolbars, and they are now functioning properly. There's a small error that I fixed, but it was a bit hard to detect. I believe that I got them all, but there is a change that I missed a couple, and unless explicitly used, it won't pop up. The error is that variables now need to be properly declared using either let or var; if not, the vue3-sfc-loader will throw an error. A lot of variables were missing that let/var. |
|
But I went through all the buttons and inputs and it didn't show up again, so I think that the k-toolbars are in the clear. |
There was a problem hiding this comment.
@HeriLFIU, excellent how it's shaping up and having vue3-sfc-loader.
I've made some comments, trivial stuff though. Other than that, quoting that you pointed out:
The error is that variables now need to be properly declared using either let or var; if not, the vue3-sfc-loader will throw an error. A lot of variables were missing that let/var.
This part of non declared vars, have they been handled in other PRs or is this still future work that needs to be done? Let's keep that in mind before marking this PR ready and then we can try to land the changes. Thanks, Heriberto.
|
@viniarck Yep, I am handling them in the individual PRs for each napp. |
|
@viniarck Also, the k-toolbars only had a few errors, which meant they were easy to fix, but the CSS for some of the info panels seems to break for some reason, and I'm trying to find out why. |
Excellent. The ones that could've got merged and wouldn't break anything I went ahead and merged them. When this PR here is ready for review, I recommend you to also list in the description the other depends PR/issues that also need to land just so we make sure to merge them all together, to avoid potentially leaving something behind or potentially broken on |
Right. |
HeriLFIU
force-pushed
the
UI/vue3sfcloaderModifications
branch
from
90e039f to
e03b62d
Compare
HeriLFIU
force-pushed
the
UI/vue3sfcloaderModifications
branch
from
e03b62d to
add1c9c
Compare
…ents tried to modify the prop and its read only data
…give it an integer value.
HeriLFIU
force-pushed
the
UI/vue3sfcloaderModifications
branch
from
d0b9dde to
0b896cc
Compare
There was a problem hiding this comment.
@HeriLFIU, great work, fantastic contribution. It's almost there. I'll leave the main review to @rmotitsuki, other than that, nice to see vue3-sfc-loader being added without having to change its source code too.
Good compromise supporting only .vue files from now on. But, regarding this point:
- Kytos web dev developer rst guide needs to be updated about .kytos being replaced with .vue https://github.com/kytos-ng/kytos/blob/master/docs/developer/web-ui.rst
- We'll need to merge this PR and related ones like Replaced search for .kytos files with .vue kytos#508 right after each other to avoid leave things broken in development when pulling the
masterbranch - Finally, we'll also need PRs in the NApps renaming .kytos files to .vue files, which will also need to land right after and together with the other related PRs.
We can start leaving the PRs pre-approved and then strategically merge when we got them all ready. Same idea as we're doing with your customClass PRs. Please sync with @rmotitsuki to prioritize the reviews and coordinate this, we're close to holiday season, so let's try to merge this in early/mid January when it's appropriate. OK?
Once again, nicely done.
|
|
||
| const res = await fetch(url); | ||
| if (!res.ok) | ||
| throw Object.assign(new Error(url + ' ' + res.statusText), { res }); |
There was a problem hiding this comment.
Did you have the chance to see this throw in practice when developing and exploring it? I'm wondering if it ever happens if it'll be obvious to spot or not
|
There are a few tiny bugs that I found and created issues for, but I was able to switch the UI to Vue JS 3 and had no big issues or errors while using it. |
|
This is with all of the most recent fixes, there are still a couple yet to be approved and merged though. |
Closes #85
Closes #105
Summary
Replaced httpvueloader with vue3-sfc-loader.
Mef-eline was trying to access and change the value of a prop within
k-accordion-itemthrough references, and props are read only. Because of this, the prop was switched for a data element since no component was using the prop functionality and the data element can be modified.Mef-eline was trying to give the prop named value from
k-inputan integer value. This prop was typed and expected a string; because of this, the type specification was removed from the prop, and now it can accept any value.Enabled the ability to disable
k-inputfor text display purposes only.Local Tests
To test the new additions all of the UI elements were used.
Note
Replace
.kytosfile extension with.vue.Incorporate changes from drafts in other napps for vue3-sfc-loader compatibility.