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

Vue3 Migration #200

Merged
merged 62 commits into from
May 26, 2023
Merged

Vue3 Migration #200

merged 62 commits into from
May 26, 2023

Conversation

CommanderStorm
Copy link
Member

@CommanderStorm CommanderStorm commented Aug 25, 2022

This PR migrates the frontend to Vue3:
vue-transition-1024x521-1

The good part:

  • The new build system is HMR ready ⇒ Changes show up automatically in about 50ms, without any user input.
  • The new buid system includes out of the gates multiple .env/request-proxying. ⇒ no more config.js, or changes, that get accidentally committed.
  • Performance seems to be on par with the old version from what I can initially see. However, I have not yet looked into the details page. This is where I expect to see the most difference in performance (some aspects positive, some negative)
  • Instead of having one large view, I have split features into smaller components if I expect that this improves maintainability.

Caveats:

  • I was unable to find a 1:1 replacement for the current 18n approach. I will have to look further into this
  • The current approach to the dark mode does not translate well. I have to look into this further…
  • I will have to look into browser support and polyfills
  • Somewhere in the middle my commit messages are meh, but I am not rebasing git, because of the likelyhood new errors would be introduced.

Things, this PR won't fix:

  • Until now the frontend was untested, while there is a large potential there, I think, this should be another PR.

How to test?

cd server
cargo run
cd webclient
npm run dev

To-dos:

  • initial migration
  • Migrate / (“main”)
  • Migrate /api
    • apply postcss stlyle transforms for the assets
  • Migrate /about/:id
  • Migrate /search
  • Migrate search bar
  • Migrate /view/:id (“details”)
    • Test that every subfeature works as expected
  • Migrate feedback
    • Test that every subfeature works as expected
  • initial deployment (vue3.nav.tum.de 😉)
  • tune the performance
    • chunking-strategy
    • look if tree shaking is happening
    • re-check if caching works as expected
  • update the documentation
  • re-add a sitemap
  • look into polyfills

Resolves #33
Resolves #213 (as this includes proper polyfills and does not depend on structuredClone)

@CommanderStorm CommanderStorm self-assigned this Aug 25, 2022
@CommanderStorm CommanderStorm added the frontend Related to the frontend label Aug 25, 2022
@CommanderStorm CommanderStorm mentioned this pull request Sep 7, 2022
5 tasks
@CommanderStorm CommanderStorm force-pushed the vue3 branch 4 times, most recently from f9721e6 to 733d487 Compare October 2, 2022 21:23
@CommanderStorm CommanderStorm force-pushed the vue3 branch 2 times, most recently from bee85e2 to ccaf6ad Compare October 8, 2022 18:20
@octycs octycs mentioned this pull request Oct 19, 2022
This was referenced Nov 9, 2022
@CommanderStorm CommanderStorm force-pushed the vue3 branch 2 times, most recently from ff6bde7 to 52b3660 Compare January 5, 2023 20:41
@CommanderStorm CommanderStorm force-pushed the vue3 branch 2 times, most recently from b5d771d to 2fc873c Compare January 22, 2023 22:53
@CommanderStorm CommanderStorm force-pushed the vue3 branch 3 times, most recently from 93db98a to 663607a Compare March 11, 2023 12:41
CommanderStorm and others added 3 commits May 24, 2023 03:12
…avigation.

The problem is that the scroll is intiated before the components are updated. If the previous page was shorter than the next one, the browser might not scroll then.
I am not sure exactly why this breaks now and didn't break in the vue2 version, but I think it is because Vue router is now async. The solution with a resizeObserver requires the least changes. Else we would need to add scrolling code to all views.
@octycs
Copy link
Contributor

octycs commented May 24, 2023

With the resizeObserver I hope the scrolling problems are sufficiently fixed now. There is still a slight glitch when navigation with the browser forward button from a page at position >0 to a page scrolled to 0. In this case it scrolls to a slightly lower position first until the component is updated. I didn't investigate this, because it might be less of a problem as soon as fading is implemented again.

I tried to implement fading by the way described in the vue-router documentation, but it causes several bugs (probably due to some async problems). We can, in order to get this finally forward, make this a new PR.

One thing I noticed for navigation is that the page is now reloaded when moving from back from an external page (Try going to MI, click on the IRIS link and then back. This is on Firefox/Chrome for me). I don't like it too much, but it's comparatively minor and probably not worth it to investigate or trivial to change.

The only thing that we might want to address before merging is this bug I can see in Chrome only (icon line widths scaled by a factor of 2):
image
Can you reproduce that?

Edit: I just noticed another bug:
Because of the lang="de" in index.html, the language used for fetching data is always de

@CommanderStorm
Copy link
Member Author

The only thing that we might want to address before merging is this bug I can see in Chrome only (icon line widths scaled by a factor of 2): image Can you reproduce that?

I cannot reproduce this. This icon looks identical between Firefox/Chrome and nav.tum.de/pr-200.nav.tum.de
image
image

@octycs
Copy link
Contributor

octycs commented May 24, 2023

I cannot reproduce this.

Okay, then it's probably a bug of my local Chrome version

@CommanderStorm
Copy link
Member Author

Edit: I just noticed another bug: Because of the lang="de" in index.html, the language used for fetching data is always de

Should we ignore this until we have SSR ready, or change this in the language switcher?

@CommanderStorm CommanderStorm merged commit bf5154f into main May 26, 2023
@CommanderStorm CommanderStorm deleted the vue3 branch May 26, 2023 14:52
github-actions bot added a commit that referenced this pull request May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend Related to the frontend help wanted Extra attention is needed
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Firefox esr is not supported [Bug] migrate webclient to Vue-3
2 participants