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

ui/ops: upgrade to Vue3, Vuetify3, vue-router4 and other required updates #261

Merged
merged 139 commits into from
Sep 3, 2024

Conversation

mattnathan
Copy link
Contributor

This PR includes all the changes needed to convert the SC BOS Ops UI from vue2 to vue3. Each commit in this PR takes one small step towards that goal - there are a lot of steps.

My development process followed these phases:

  1. Update versions and bootstrap code
  2. Use automated tooling (eslint-plugin-vuetify mostly) to fix specific migration problems one at a time, test it using eslint directly or using eslint-nibble to target specific checks.
  3. Keep making changes until vite would run the app and it would load in the browser - this was a good day
  4. Run the vanti-ugs example project with as many features turned on as possible and run the ui clicking on pages and checking the console - fix any warnings/errors you see in the console (that weren't there already)
  5. Do a side-by-side comparison of every page, popup, interaction, component, etc of the main branch and this branch to check for differences
  6. For each difference, check (via text search) how many other places that pattern/property/etc was used and fix it
  7. Fix any related style relating to the component/page
  8. Do a side-by-side comparison of this branch vs deployed ui at EW for all pages/popups/features available

The new styles do not match 100% with the old ones, they are not pixel perfect. Where decisions around layout appeared to be arbitrary I've erred on the side of fewer changes. Some examples: icon sizes are slightly smaller in some places. Font size too. Some icons have changed where the default vuetify ones were used. Table pagination has some more buttons. Select boxes and inputs have a different background colour. And so on.

I'm not expecting anyone to actually do a code review of the components/js, however the setup code should be looked at:

  • vite.conf.js
  • eslint.cjs
  • src/main.js
  • src/plugins/*
  • src/routes/**/route.js
  • there may be others

Instead I'd like reviewers to check out this branch and run the app against sites and local setups they know how to test with. This is a huge change that has likely broken something that I haven't spotted yet. I haven't been able to test the new version with all of the environments and sites out there, this is where I need help.

…te-plugin-vuetify

Remove unplugin-vue-components
We couldn't do this before because vuetify was using discontinued features
This is definitely not working or styled how we want it
…t a time

This will be removed after we have no more eslint errors
Manual changes required:
- anywhere that looked like `v-bind={...props1, ...props2}`
- special cases for when $attrs is used to add defineOptions
- whitespace left over from the auto tool
- some bug fixes where we were using `attr` instead of `attrs`
- adjustment of v-snackbar #action->#actions

Disable (for now) prop shadowing for these cases
The lint still fails for a lot of cases, but these are the automatic changes.
Manual fixes include whitespace changes only.
Where it was a simple wrapper around title and subtitle, etc. we delete the wrapper.
Where it is used to layout child components (via flex usually) we replace with a div.

This likely has broken some layout. When everything builds again we'll revisit to fix things.
In the future we can likely replace most with use of the prepend-icon prop.

There was one case in groups where the icon wasn't actually in a v-list-item so this has been fixed.
This required restructuring the tabs by hand.
Mostly it's just deleted, sometimes it's replaced with v-list-group
Manual steps include:
- fix whitespace issues
- where multiple variant attrs were used, pick one
We can add them back in when we get to ui fixes
Copy link
Contributor

@deanredfern26 deanredfern26 left a comment

Choose a reason for hiding this comment

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

Couldn't see any issues at OCW, at EW I noticed the headings in the modes component are cut off. Can't see any other issues

image

@mattnathan
Copy link
Contributor Author

Couldn't see any issues at OCW, at EW I noticed the headings in the modes component are cut off. Can't see any other issues

image

OK, that should be fixed now. I also noticed some issues with the UDMI card which I've fixed.

image

This is caused by yarn not respecting the ,npmrc file when in workspace mode.
Copy link
Contributor

@hexaglow hexaglow left a comment

Choose a reason for hiding this comment

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

Visually looks good, I wouldn't notice any differences if you didn't point them out.

One issue - when running yarn install with Node 18 I encountered the following:

yarn install v1.22.22
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
warning chart.js@4.4.2: The engine "pnpm" appears to be invalid.
error glob@11.0.0: The engine "node" is incompatible with this module. Expected version "20 || >=22". Got "18.20.0"
error Found incompatible module.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

But the build-sc-bos action still specifies Node 18. If Node 20 is required then this needs to be updated.

@mattnathan
Copy link
Contributor Author

Visually looks good, I wouldn't notice any differences if you didn't point them out.

One issue - when running yarn install with Node 18 I encountered the following:

yarn install v1.22.22
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
warning chart.js@4.4.2: The engine "pnpm" appears to be invalid.
error glob@11.0.0: The engine "node" is incompatible with this module. Expected version "20 || >=22". Got "18.20.0"
error Found incompatible module.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

But the build-sc-bos action still specifies Node 18. If Node 20 is required then this needs to be updated.

I'll update to the current release (22.x)

@mattnathan mattnathan merged commit 2689ab7 into main Sep 3, 2024
2 checks passed
@mattnathan mattnathan deleted the vue3 branch September 3, 2024 12:56
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.

3 participants