-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Let apps toggle an unread counter on app icons #26939
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
Conversation
| export const setUp = () => { | ||
|
|
||
| Object.assign(OC, { | ||
| setNavigationCounter(id, counter) { |
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'm a bit undecided on the approach to take here, maybe we should rather listen to an event that might be emitted then by the apps through the event bus?
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 advocate against the global variable approach, at least form an app API perspective. We can hide this in yet another @nextcloud/xyz package or add it to an existing one. Then we can also later on change the way this action propagates from an app to the server component.
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.
OK, then lets keep the OC.setNavigationCounter for use in the additional package then which we then can use for offering a stable API. Unfortunately I don't think we have any fitting package for that already so it would need an additional one.
|
For reference what I also said in the chat:
In the notifications app we provide 2 icons, 1 regular and 1 with dot. We could do the same with the app icons as well, and standardize the position and size via recommendation. That way we can also achieve the cut-out border for the dot which is difficult to do via CSS because we have a fade in the header. |
|
what's the state here? moving to 23? |
7db0abe to
8818011
Compare
8818011 to
d563317
Compare
|
Ready review, we'd still need this for 22. |
|
CI is unhappy |
489f341 to
a73b6aa
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.
👍 design-wise :)
a73b6aa to
a0f081d
Compare
|
Pushed another fix for the tests, waiting for CI to pass |
|
CI is unhappy |
Co-authored-by: Louis Chemineau <louis@chmn.me> Signed-off-by: Julius Härtl <jus@bitgrid.net>
a0f081d to
a942364
Compare
The count value is currently hidden as we don't want to clutter the UI too much, but might be useful at some point.
It can either be updated during page load through the navigation manager
$navigationManager->setUnreadCounter('activity', 10);or through JavaScript by callingOC.setNavigationCounter('deck', 123)