-
Notifications
You must be signed in to change notification settings - Fork 159
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
feat: app store app #11302
feat: app store app #11302
Conversation
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
fc8704a
to
04dd2d2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
8a49fe3
to
43649da
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it! 😍
What do you think about debouncing the initial loading state a bit? I always see this blue box showing up for a split-second.
packages/web-app-admin-settings/src/components/Groups/GroupsList.vue
Outdated
Show resolved
Hide resolved
packages/web-app-admin-settings/src/components/Users/UsersList.vue
Outdated
Show resolved
Hide resolved
…'t make assumptions
Went back to using the good old app-loading-spinner component. Showing text content feels just like too complex for a simple loading state. |
@@ -0,0 +1,113 @@ | |||
<template> | |||
<div class="app-list oc-mb-m"> | |||
<h2 class="oc-mt-rm" v-text="$gettext('App Store')" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest adding a link to the install guides for apps here? I naively thought I'd install them directly into my instance and was a bit surprised when it "just" downloaded a zip file 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Phase 1 is discovery
, phase 2 is easy install
, we need backend features for phase 2.
Do you have a good idea where to put such a link? It would be helpful indeed...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess an ℹ️ icon next to the heading with a tooltip that provides a link to https://owncloud.dev/services/web/#loading-applications would suffice for now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point it looks pretty mature to me 👍
I suggested to add a link to our docs, so the user knows how to install a an app.
Maybe we can add a context helper next to the download buttons or a link with text (How to install) under them.
Approving anyways
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super awesome! 🚀
Quality Gate passedIssues Measures |
Description
New app store app
Related Issue
Motivation and Context
Make Web great again
Types of changes
Open Tasks