Skip to content

Conversation

@smarinier
Copy link
Contributor

(All this came mainly from my previous dev)

  • Applications use their real name and their icon, (Material UI "API" icon is used by default to avoid ugly holes in list)
  • Apis are splitted into two lists according to their "permanent in Nextcloud" status
  • subset of apis (eg. Provisioning, or you may try with Talk/spreed application too) are expandable choices
  • use a router : this allow permanent url (bug Update URL when opening elements #34)
  • the store is necessary with this structured array of data for all apis
  • the welcome page has be redesigned (inspired by Talk welcome) . I propose to invite devs to build openapi.json, so that this app might be fully usefull

…and store, new welcome

Signed-off-by: Sebastien Marinier <[email protected]>
@provokateurin
Copy link
Member

Thanks a lot already, I will review next week!

@provokateurin provokateurin linked an issue Nov 8, 2024 that may be closed by this pull request
@nickvergessen nickvergessen added the enhancement New feature or request label Nov 8, 2024
Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

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

In dark mode the app icons are not white:
image
The core logo also isn't centered for some reason.

@smarinier
Copy link
Contributor Author

The core logo also isn't centered for some reason.
Sorry, but it's centered on my Nextcloud. Could you help me to find why it's not on yours ?

@provokateurin
Copy link
Member

Sorry, but it's centered on my Nextcloud. Could you help me to find why it's not on yours ?

I will take a look, but it's only a minor issue compared to the other things I found, so we can deal with it later.

@smarinier
Copy link
Contributor Author

Hi @provokateurin, is everything fine or do I miss something ?

@provokateurin
Copy link
Member

Nothing wrong here, just busy with other tasks. I'll review it soon :)

@provokateurin
Copy link
Member

Sorry I forgot about this 😓

Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

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

Really good work!

One additional thing I noticed but am not sure if/how it can be fixed:
When I expand a navigation item and then click on a completely different one the first item is collapsed again.
Is it possible to fix this in some way? I think it is fine UX wise, but the UI jumps around due to the collapsing item which is a bit unpleasant.

@smarinier
Copy link
Contributor Author

Really good work!

One additional thing I noticed but am not sure if/how it can be fixed: When I expand a navigation item and then click on a completely different one the first item is collapsed again. Is it possible to fix this in some way? I think it is fine UX wise, but the UI jumps around due to the collapsing item which is a bit unpleasant.

Done also (listen to events, keep a manual state)

- don't use backend generic home route
- use 'specs' and apiSpecs
- child apis draw their icon in 16
- keep open states

Signed-off-by: Sebastien Marinier <[email protected]>
Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

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

Thanks again! This is so much better than what we had before 😍

@provokateurin
Copy link
Member

The OC::$SERVERROOT psalm failure you have to ignore, but the other ones have to be fixed

@smarinier
Copy link
Contributor Author

The OC::$SERVERROOT psalm failure you have to ignore, but the other ones have to be fixed

-> need to upgrade the AppManager interface up to NC 29 (wich is minimum required)
-> use psalm annotation for SERVERROOT

@provokateurin provokateurin merged commit 016d8d6 into nextcloud:main Nov 26, 2024
12 checks passed
@provokateurin
Copy link
Member

Thanks again! I'm going to make a new release in the next days, so everyone will get these nice improvements.

@smarinier
Copy link
Contributor Author

Thanks you Kate!

i happened to see a recent post on LinkedIn concerning this app (https://www.linkedin.com/posts/nextcloud-gmbh_integrate-nextcloud-into-your-app-with-the-activity-7265793671776608256-xEn-?utm_source=share&utm_medium=member_desktop). I'm glad to have been able to contribute!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update URL when opening elements

3 participants