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

Upgrade to Vuetify 2 #886

Closed
wants to merge 1 commit into from
Closed

Conversation

benmccann
Copy link
Contributor

@benmccann benmccann commented Jan 12, 2021

This is makes most of the necessary changes. A few of the styles might need to be tweaked since some of the default colors have changed

I'm hoping we can get this in before it ends up with too many merge conflicts

@lastzero
Copy link
Member

Vuetify 2 is not just an update, it is based on Material Design 2 thus changing the design completely. Before we get into this, we really need to implement some of the feature requests on our roadmap first.

@benmccann benmccann force-pushed the vuetify-2 branch 16 times, most recently from bbd5eb6 to 65df41f Compare January 20, 2021 05:37
@benmccann benmccann marked this pull request as ready for review January 20, 2021 05:42
@benmccann
Copy link
Contributor Author

@lastzero I've gotten this to a pretty good place. The design really isn't much different except for some colors are a little different (e.g. the box under the album cover).

I understand that this won't be a big improvement for users, but it would help me a lot in working on more important features because the Vuetify 2 docs are searchable whereas the Vuetify 1 docs are not. More importantly, I've already gotten this PR to basically a mergable state and I'd really hate for it to go stale only to have to redo it a few months from now. I can coordinate with you before starting a new PR in the future and try to work on more impactful stuff, but do hope we can get this in. I'm happy to make any changes that you'd like or that other users request as a result of this PR.

@lastzero
Copy link
Member

Well, they had a search for Vuetify 1.5 - probably still around somewhere :)

If we upgrade to Material Design 2, we should also update the design of our web site, the docs and the backend we're currently building. Any idea when I can finally get some sleep? 😂

@benmccann benmccann force-pushed the vuetify-2 branch 3 times, most recently from a64477d to ce89bed Compare January 20, 2021 14:46
@benmccann
Copy link
Contributor Author

The application really doesn't look much different at all. I've attached a screenshot below. I don't think anyone would say that the app and docs were similarly themed before and now they're not. The main thing I notice is that some colors have changed a little. There's also a little more spacing in the nav.

Screenshot from 2021-01-20 06-00-51

I'm assuming we'd have to upgrade at some point. Vuetify 1.5 reached end of life on July 31st, 2020 and is no longer actively maintained. It's harder to ask for help, report bug fixes, etc. since we're on 1.5 which has made it harder for me to ramp up on the codebase. I really hope that this PR would save you work over the long-run by getting a task out of the way. Hopefully this PR wouldn't be that much work for you in the short-term either - just a bit of poking around and giving me any feedback on changes you'd like if any. I'm happy to support this after it goes in as well if you later discover something that needs to be tweaked as a result.

@lastzero
Copy link
Member

Appreciate your initiative, and I agree with most of what you say. Need a couple of hours to test this in detail before we can merge it. I'll do my best to get it done as soon as possible. Need to take a look at bug reports first. My 40th birthday is on Friday as well, so I'll take two or three days off. When I'm back, my inbox and GitHub issues will be flooded with requests. Just to give you an idea :)

Spacing in fact was a major issue from what I remember. Logo and nav items are not properly aligned anymore as when I tested it last time. This probably can be fixed, just takes time which is an extremely scarce resource for us right now.

@benmccann benmccann force-pushed the vuetify-2 branch 2 times, most recently from 60a2cb7 to 990d444 Compare January 20, 2021 21:37
@benmccann
Copy link
Contributor Author

Thanks! I appreciate that.

Logo and nav items should be properly aligned, but I expect there will be some smaller things here and there. I'm happy to make whatever changes you'd like to suggest when you get a chance to take a look

40 is a big one, so you deserve some time off! Enjoy! 😄

@lastzero
Copy link
Member

20210124-113154-Berlin-Germany-2021-3aw

@benmccann
Copy link
Contributor Author

looks good!! 😄 🍰 carrot cake?

@lastzero
Copy link
Member

Yes, was delicious 😋

As expected, returning to work was intense - had to zoom out to take a screenshot of today's commit log:

today

@benmccann
Copy link
Contributor Author

ooh, wow! lots of good stuff happening! I was nervous about rebasing this, but luckily only one file conflicted. for now... 😀

@flexage
Copy link

flexage commented Mar 7, 2021

I'd also like to see this merged.

On a performance level, Vuetify 2 comes with a Virtual Scroller Component built-in (https://vuetifyjs.com/en/components/virtual-scroller/)

This would really help with performance degradation when having lots of photo results loaded.

To illustrate, here's a real-world scenario I encountered just yesterday:

  • I have recently performed a Google Photos Takeout containing 40,000 photos.
  • After importing, ~10,000 of them ended up in "Review"
  • I start to perform a review, taking the strategy of working through them year by year, to provide milestones as to completion
  • Begin to mark photos for archival, scroll down to load in the next batch of photos, mark photo's for archival repeat several times
  • Notice that as the loaded list of results continues to grow, the UI starts to lag - Next photos become slower to render (beginning with just a couple of seconds, growing to 15+ seconds), the "marking" of photos starts to be laggy, things worsen as more photos are loaded, Chrome begins to show warnings about the page being unresponsive.

Not a great experience, and something we would want to handle.

I've come across this issue before in Vue, and in my experience the most likely cause is related to the performance of both the Vue renderer and, to a lesser degree, the browser renderer.

The Virtual Scroller component helps get around this by only rendering items that are currently within the viewport, swapping out items on scroll as necessary, cutting down the need to re-render potentially thousands of items on every change to the dataset, and keeping the DOM light.

The principal is similar to that of the Recycler View that Android has had for many years now (along with equivalents on other platforms).

If sticking with Vuetify 1.5, there are third-party vue components that could be used in place of the Virtual Scroller component to achieve much the same thing, but I would argue that it makes sense to stick with the Vuetify components where possible, unless there is a compelling reason to do so, hence my +1 for Vuetify 2.x.

I would also agree with @benmccann that a Vuetify version upgrade does not necessitate an upgrade to the visual aspect of the app, or surrounding sites, docs.

Changes to the UI should be a considered effort and not driven by upgrades to dependancies unless the dependancy either enforces the need to change the UI, or if the changes need after the dependancy upgrade are simply too much work to perform just to maintain UI homeostasis.

I would be happy to investigate implementing the Virtual Scroller component, and work towards a stable pull-request (after all, this is a feature that would benefit all, including me), however the direction I would take on the implementation of this feature is dependant on this PR being merged, or not.

@lastzero lastzero self-assigned this Apr 11, 2021
@isaac84
Copy link

isaac84 commented Nov 2, 2021

Looks like Vuetify 3 is scheduled for release in February 2022. So it might just be worth skipping 2 all together if it's going to mean styling rework for an upgrade (no offence @benmccann). It's probably more work than upgrading to 2 though. Just my 2 cents.

@lastzero
Copy link
Member

lastzero commented Nov 2, 2021

Agreed. Right now, we sadly don't have even a second of time for either. Should use Vue 3 as well. Working on multi user right now. Roadmap is stuffed and our funding goal still not reached after 4 years. Could work faster if we had more resources. Funding has priority before adding more work to our list.

@isaac84
Copy link

isaac84 commented Nov 2, 2021

I'm tempted to take a look when I get a break from my day job over Xmas. I'd be learning Go at the same time as I come from a mostly Java background.

@lastzero
Copy link
Member

lastzero commented Nov 2, 2021

Note we have additional themes, JS unit tests & UI acceptance tests that require testing and probably refactoring. Needs to be done before we can merge a major UI upgrade and release it.

@isaac84
Copy link

isaac84 commented Nov 2, 2021

Understood, that's the only way to keep the quality high so I am all for that.

@laterwet laterwet mentioned this pull request Jan 17, 2022
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

None yet

5 participants