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.

Sign up for GitHub

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

Jump to bottom

feat: Refactor app & account management UI code #44092

Merged
merged 11 commits into from
Mar 11, 2024
Merged

feat: Refactor app & account management UI code #44092

merged 11 commits into from
Mar 11, 2024

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Mar 8, 2024 •
edited
Loading

Summary

  1. Separate app store from account management
  2. Split app and account management into better maintainable smaller pieces
    • Separate app content and app navigation component
    • Also split app store sidebar into own components for each tab.
    • The details tab now includes the same information like the sidebar on apps.nextcloud.com
  3. Everything I needed to touch more then copy paste was refactored to TypeScript
  4. Start using Pinia for the app store (currently only the navigation -> rest will be follow up)
  5. Fix icon color broken in app-store #42998 by using inline icons for the app navigation

Screenshots

Screenshot 2024-03-08 at 18-42-21 Office text - Apps - Nextcloud
Screenshot 2024-03-08 at 18-42-14 Office text - Apps - Nextcloud
Screenshot 2024-03-08 at 18-42-07 Office text - Apps - Nextcloud
Screenshot 2024-03-08 at 18-34-40 Office text - Apps - Nextcloud
Screenshot 2024-03-08 at 18-32-47 Integration - Apps - Nextcloud

Checklist

@susnux susnux added this to the Nextcloud 29 milestone Mar 8, 2024
@susnux susnux marked this pull request as ready for review March 8, 2024 19:46
@susnux susnux force-pushed the feat/refactor-app-account-management branch from a411cb2 to 58595d4 Compare March 8, 2024 19:46
emoral435
emoral435 reviewed Mar 10, 2024
View reviewed changes
Copy link
Contributor

@emoral435 emoral435 left a comment

Choose a reason for hiding this comment

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

Really great PR, loved seeing typeof === 'undefined' 👀

apps/settings/src/components/AppStoreSidebar/AppReleasesTab.vue Outdated Show resolved Hide resolved
apps/settings/src/composables/useAppIcon.ts Show resolved Hide resolved
apps/settings/src/views/AppStore.vue Show resolved Hide resolved
@susnux susnux force-pushed the feat/refactor-app-account-management branch from 58595d4 to cbad56f Compare March 10, 2024 20:33
@susnux susnux requested a review from emoral435 March 10, 2024 20:33
emoral435
emoral435 approved these changes Mar 10, 2024
View reviewed changes
Copy link
Contributor

@emoral435 emoral435 left a comment

Choose a reason for hiding this comment

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

really nice :)

artonge
artonge approved these changes Mar 11, 2024
View reviewed changes
apps/settings/src/main-apps-users-management.ts Outdated Show resolved Hide resolved
apps/settings/src/views/AppStore.vue Show resolved Hide resolved
szaimen
szaimen previously requested changes Mar 11, 2024
View reviewed changes
Copy link
Contributor

@szaimen szaimen left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks great in general, but would be good if possible to have the button to download and enable in the header and not hidden in a tab.
Dirty mockup:
image

@susnux
Copy link
Contributor Author

susnux commented Mar 11, 2024

Looks great in general, but would be good if possible to have the button to download and enable in the header and not hidden in a tab.

@szaimen I fully agree, this is a leftover from the old / current design. But I would really like to do that in a follow up. This PR is already too huge. Is that ok for you?

@szaimen
Copy link
Contributor

szaimen commented Mar 11, 2024

@szaimen I fully agree, this is a leftover from the old / current design. But I would really like to do that in a follow up. This PR is already too huge. Is that ok for you?

yes, fine for me. Shall I create an issue so that it doesnt get lost or do you want?

@susnux
Copy link
Contributor Author

susnux commented Mar 11, 2024

yes, fine for me. Shall I create an issue so that it doesnt get lost or do you want?

I will do

@susnux susnux mentioned this pull request Mar 11, 2024
Open
@susnux susnux force-pushed the feat/refactor-app-account-management branch from cbad56f to 7be174e Compare March 11, 2024 12:50
@susnux susnux requested a review from szaimen March 11, 2024 12:55
@susnux susnux force-pushed the feat/refactor-app-account-management branch from 7be174e to 68c63f8 Compare March 11, 2024 13:07
@szaimen szaimen dismissed their stale review March 11, 2024 13:11

Will be done in follow-up

@susnux susnux force-pushed the feat/refactor-app-account-management branch from 68c63f8 to bc1a3bd Compare March 11, 2024 13:49
@susnux
feat(settings): Refactor routing code to use named router views
14bdc11 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
feat(settings): Split appstore code into better maintainable pieces
e678b77 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
fix: Get rid of PrefixMixin
289e43a 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
feat: Make appstore sidebar tabs standalone components
84cb04f 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
feat: Add composable to fetch app icon as SVG for inline use
5f19ace 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
feat(settings): Refactor appstore to use Pinia
2b794b4 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
feat(settings): Split account management and app store views into chunks
1100e90 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
feat(settings): Split account management into navigation and content
30d5b02 
The should ease the maintenance of it due to reduced complexity.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
fix(settings): Always open the details tab for selected app
cbdadba 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
fix(settings): Save account management settings in local storage
78bcab1 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
chore: Compile assets
fc73bd0 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux force-pushed the feat/refactor-app-account-management branch from bc1a3bd to fc73bd0 Compare March 11, 2024 15:03
@susnux susnux enabled auto-merge March 11, 2024 15:04
@susnux susnux merged commit 8ad8280 into master Mar 11, 2024
98 checks passed
@susnux susnux deleted the feat/refactor-app-account-management branch March 11, 2024 15:21
@Altahrim Altahrim mentioned this pull request Mar 12, 2024
Merged
@ShGKme ShGKme mentioned this pull request Mar 16, 2024
Merged
4 tasks
@gp994
Copy link

gp994 commented Mar 29, 2024

I've just upgraded Nextcloud to 28.0.4 but the bug on Safari is still present.
In which Version will it be fixed?
Thanks a lot

@susnux
Copy link
Contributor Author

susnux commented Mar 29, 2024

@gp994 this one is not fixing safari. This bug was fixed for Nextcloud 29 here: #44236
And a backport is pending for Nextcloud 28 here: #44327

@Pytal Pytal mentioned this pull request Sep 10, 2024
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artonge artonge artonge approved these changes

@emoral435 emoral435 emoral435 approved these changes

@Pytal Pytal Awaiting requested review from Pytal

@szaimen szaimen Awaiting requested review from szaimen

Assignees
No one assigned
Labels
enhancement feature: apps management
Projects
None yet
Milestone
Nextcloud 29
5 participants
@susnux @szaimen @gp994 @artonge @emoral435