search

marius-wieschollek / passwords

A simple, yet feature rich password manager for Nextcloud
GNU Affero General Public License v3.0
201 stars 39 forks source link

Use @nextcloud/vue #439

Closed raimund-schluessler closed 1 year ago

raimund-schluessler commented 2 years ago

Current Status The app uses many custom components for which there are replacements in the @nextcloud/vue component bundle, e.g.

Feature Description Consider using the components provided by @nextcloud/vue, see https://github.com/nextcloud/nextcloud-vue.

Additional context Using the components from @nextcloud/vue would reduce the maintenance effort for this app, since the components are then maintained by the community and Nextcloud GmbH. It would furthermore bring the look and feel closer to other Nextcloud app which share the same components.

In case you like the idea I can start sending some PRs to get this going.

marius-wieschollek commented 2 years ago

If you would like to work on this, i'd be happy to look at any PR. But be aware that migrating to nextcloud vue won't be easy.

raimund-schluessler commented 2 years ago

@marius-wieschollek Do you have a particular reason why you think it won't be easy? I thought about starting small and replacing components like Breadcrumbs or Actions and only later on replacing app containers such as AppContent or AppSidebar (which indeed might be a bit of work). Does that make sense to you?

marius-wieschollek commented 1 year ago

At the time when this ticket was opened, my main concern was the icons. Passwords uses FontAwesme 4.7 and NC only supported it's own tiny iconset when i last checked. By now, NC recommends the vue material design icons, and nextcloud/vue also supports custom icons everywhere.

I have already begun to move the app over to nextcloud/vue and hope that this will make support for future NC versions easier.

The only risk that comes with this is that i don't know how compatible nextcloud/vue is with different versions of NC. I don't want to drop support for a version just because i can't fix the design.

raimund-schluessler commented 1 year ago

At the time when this ticket was opened, my main concern was the icons. Passwords uses FontAwesme 4.7 and NC only supported it's own tiny iconset when i last checked. By now, NC recommends the vue material design icons, and nextcloud/vue also supports custom icons everywhere.

Yes, every component should support material design icons by now. The variety of icons in the package used is quite good, there should be a replacement for every icon.

I have already begun to move the app over to nextcloud/vue and hope that this will make support for future NC versions easier.

Great to hear this. I think it should make it easier, since you don't need to maintain these components anymore. Let me know if you need help.

The only risk that comes with this is that i don't know how compatible nextcloud/vue is with different versions of NC. I don't want to drop support for a version just because i can't fix the design.

This is usually not a big issue. However, the update to NC25 unfortunately brought a breaking design change in server, which lead to a breaking change in the components as well. Server v25 requires nextcloud/vue@7 which doesn't work with server <25 anymore. Most apps I know either have to maintain two branches, one for server23/24 and one for later versions; or they dropped support for 23/24 completely and only offer a version for 25 (which should then be compatible with later server versions as well). Which approach is doable depends on how much time one has to support different versions, I guess.

marius-wieschollek commented 1 year ago

First batch of changes is part of 2023.1.0. One thing i didn't know before was how much bloat the nextcloud vue components have. See https://help.nextcloud.com/t/how-to-deal-with-nextcloud-vue-bloat/153227 for details.

I was able to keep the initial app size acceptable by removing and replacing other dependencies and making a lot more use of asynchronous loading to load components only when needed.

IMO @nextcloud/vue really needs to go on a diet. 180kb for a list entry isn't normal

ghost commented 1 year ago

Is the entire app transferred to Nextcloud Vue?

marius-wieschollek commented 1 year ago

Anything that profits from the migration has been migrated.

ghost commented 1 year ago

The Share section in the sidebar could benefit from this. E.g. sharing with groups, circles, etc.

marius-wieschollek commented 1 year ago

Nextcloud vue is just ui. There won't be new sharing functionality just because i use any of the components.

I used the select component and had to do most of the styling myself as nextcloud/vue apparently provided no styling for this component for some reason. And the Nextcloud list item components add a lot of file size to the app, but all they can do is show the list. There is no benefit to using them.