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

Port bootstrap page to Typescript #1573

Merged
merged 1 commit into from
Apr 26, 2023
Merged

Port bootstrap page to Typescript #1573

merged 1 commit into from
Apr 26, 2023

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Apr 22, 2023

This one contains more changes than the status page. I started to make use of the new tooling to port the page to Vue. Now it dynamically refreshes itself when the selected data changes, instead of reloading the whole page. It still updates the URL though, so Go back in the browser works as expected and the URL can be shared to display the same data selection.

@Kobzol Kobzol requested a review from rylev April 22, 2023 18:00
@Kobzol Kobzol force-pushed the npm-bootstrap branch 2 times, most recently from 722505e to 54f15d1 Compare April 22, 2023 18:15
@Kobzol
Copy link
Contributor Author

Kobzol commented Apr 23, 2023

After experimenting with it for a bit, I decided to throw away the "SPA aspect" for now. Turns out that with the bfcache optimization, it's actually much faster to go back/forward in browser history here if the page is reloaded on every data selection change. If we implemented it as an SPA, we would need to cache the downloaded data ourselves to make it as fast. That's not worth it here at this moment I think.

The code is also a bit simpler now.

Copy link
Member

@rylev rylev left a comment

Choose a reason for hiding this comment

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

Looks fine to me!

@Kobzol Kobzol merged commit c14370d into master Apr 26, 2023
@Kobzol Kobzol deleted the npm-bootstrap branch April 26, 2023 18:22
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.

2 participants