-
Notifications
You must be signed in to change notification settings - Fork 90
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
Replace AppNavigationCounter with CounterBubble #1895
Conversation
Replaces AppNavigationCounter as a more generic component. Signed-off-by: Vincent Petry <vincent@nextcloud.com>
190545b
to
b6af301
Compare
But shouldn't this be staandardized here? So a bubble can be applied to an action? |
The usage of |
Not sure how to standardize alignment in code. It would require knowing the size of the element (action, app navigation, ...) on top of which is needs to be positioned and aligned on top of it. Also need a proper container with position: relative for the positioning the work. |
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
@raimund-schluessler thanks for pointing out. I've adjusted the usages and also fixed some unrelated warnings while browsing the ListItem page in the style guide. After seeing the usages, maybe we could add a counter slot for the "Actions" element (or any of the relevent sub-elements), this way the alignment could be achieved by the action* components themselves. |
<!-- | ||
- @copyright Copyright (c) 2019 Marco Ambrosini <marcoambrosini@pm.me> | ||
- | ||
- @author Marco Ambrosini <marcoambrosini@pm.me> |
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.
@PVince81 Probably not really important, but I guess this is the wrong year and author?
Because we need a more generic bubble, like in this example:
This is from nextcloud/spreed#5534
Keep the old file to not break existing imports.
I've tested it with some local Talk code that is using AppNavigationCounter and it properly redirects the component.
The only issue I see is that the style guide doesn't show the deprecation notice and replaces the component with the name: