Skip to content

Vue3 Migration#200

Merged
CommanderStorm merged 62 commits intomainfrom
vue3
May 26, 2023
Merged

Vue3 Migration#200
CommanderStorm merged 62 commits intomainfrom
vue3

Conversation

@CommanderStorm
Copy link
Member

@CommanderStorm CommanderStorm commented Aug 25, 2022 •
edited
Loading

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
Merged
5 tasks
This was referenced Sep 26, 2022
Closed
Closed
@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
Closed
@CommanderStorm CommanderStorm mentioned this pull request Oct 19, 2022
Closed
@CommanderStorm CommanderStorm mentioned this pull request Oct 27, 2022
Closed
@octycs octycs mentioned this pull request Oct 30, 2022
Closed
3 tasks
This was referenced Nov 9, 2022
Merged
Merged
@CommanderStorm CommanderStorm mentioned this pull request Nov 25, 2022
Closed
@CommanderStorm CommanderStorm mentioned this pull request Dec 12, 2022
Merged
@CommanderStorm CommanderStorm force-pushed the vue3 branch 2 times, most recently from ff6bde7 to 52b3660 Compare January 5, 2023 20:41
@CommanderStorm CommanderStorm mentioned this pull request Jan 11, 2023
Closed
@CommanderStorm CommanderStorm force-pushed the vue3 branch 2 times, most recently from b5d771d to 2fc873c Compare January 22, 2023 22:53
@CommanderStorm CommanderStorm mentioned this pull request Jan 24, 2023
Open
@CommanderStorm CommanderStorm mentioned this pull request Feb 3, 2023
Merged
2 tasks
@CommanderStorm CommanderStorm force-pushed the vue3 branch 3 times, most recently from 93db98a to 663607a Compare March 11, 2023 12:41
@CommanderStorm
Merge branch 'main' into vue3
e5fc9c0
@CommanderStorm
Merge branch 'main' into vue3
05fa904
@CommanderStorm
added debug information to the language toggler
a6dd444
@CommanderStorm
added VITE_APP_URL to the environent variables
c65b41a
@CommanderStorm
tested a diffferent approach to introduce VITE_APP_URL into the webcl…
61f90e7 
…ient
@CommanderStorm
maybe fixed the cookies not loading properly?
c19c4c0
@CommanderStorm
renamed utils to composables
c4f7a81
@CommanderStorm
added a saveCooke() function to centralise this functionality
4d1a60b
@CommanderStorm
aded an debug alert
ef88cbc
@CommanderStorm
removed alert
76761f6
@CommanderStorm
tested a different way to scope cookies
57140b0
@CommanderStorm
removed logging statements from the code
063e281
@CommanderStorm
uptated the typescript bindings for the openapi spec
212c436
@CommanderStorm
bumped our linting suite and swagger-ui
79c4869
@CommanderStorm
Merge branch 'main' into vue3
8123be2
@CommanderStorm
Merge branch 'main' into vue3
6b758c4
@CommanderStorm
Merge branch 'main' into vue3
fe9b517
@CommanderStorm
bumped maplibre to v3 as in master
9c94c69
CommanderStorm added a commit that referenced this pull request May 24, 2023
@CommanderStorm
proposed a fix for the server-backoffing behaviour seen in #200
a0b2453
CommanderStorm and others added 3 commits May 24, 2023 03:12
@CommanderStorm
Merge branch 'main' into vue3
cd4b3ce
@CommanderStorm
Merge branch 'main' into vue3
4aa4c2f
@octycs
Add resizeObserver to scroll to y-positions > 0 on forward/backward n…
d103e61 
…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 •
edited
Loading

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

CommanderStorm commented May 24, 2023

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

CommanderStorm commented May 24, 2023

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
@github-actions
Removed the deployment for PR #200
2d9b9ab
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@octycs octycs octycs left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@CommanderStorm CommanderStorm

Labels

frontend Related to the frontend help wanted Extra attention is needed

Projects

No open projects
NavigaTUM
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

2 participants

@CommanderStorm @octycs