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.

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

Enh/Align app navigation item counter in a better way #4991

Closed
wants to merge 2 commits into from

Conversation

GVodyanov
Copy link
Contributor

☑️ Resolves

🖼️ Screenshots

🏚️ Before 🏡 After
image image

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable

susnux
susnux previously approved these changes Dec 23, 2023
@@ -314,7 +314,7 @@ Just set the `pinned` prop.
<!-- Counter and Actions -->
<div v-if="hasUtils && !editingActive"
class="app-navigation-entry__utils"
:class="{'app-navigation-entry__utils--display-actions': forceDisplayActions || menuOpenLocalValue || menuOpen }">
:class="{'app-navigation-entry__utils--display-actions': forceDisplayActions || menuOpenLocalValue || menuOpen, 'app-navigation-entry__utils--add-spacing': !hasChildren }">
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe call it --has-children or something? Just to make it more descriptive?

@susnux susnux added enhancement New feature or request feature: app-navigation Related to the app-navigation component design Design, UX, interface and interaction design labels Dec 23, 2023
Copy link
Contributor

@raimund-schluessler raimund-schluessler left a comment

Choose a reason for hiding this comment

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

This leads to the misalignment of the utils/actions on the right, as can be seen in the screenshot. This needs to be fixed, as it looks far worse than the right-aligned counter in my opinion.

@susnux susnux dismissed their stale review December 23, 2023 16:54

Overseen the visual regression

@GVodyanov
Copy link
Contributor Author

@raimund-schluessler Does this look better?
image

@raimund-schluessler
Copy link
Contributor

@raimund-schluessler Does this look better?

Yes, better. But now the counter jumps to the right when the entry is hovered:

grafik

@GVodyanov
Copy link
Contributor Author

@raimund-schluessler Does this look better?

Yes, better. But now the counter jumps to the right when the entry is hovered:

grafik

Could you please suggest how I could reproduce this?

@raimund-schluessler
Copy link
Contributor

Could you please suggest how I could reproduce this?

Go to the docs https://deploy-preview-4991--nextcloud-vue-components.netlify.app/#/Components/App%20containers/NcAppNavigation?id=ncappnavigationitem scroll to "Element with counter" and hover it.

@susnux susnux modified the milestones: 8.5.1, 8.6.0 Jan 24, 2024
@Antreesy Antreesy modified the milestones: 8.6.0, 8.6.1 Jan 30, 2024
@ShGKme ShGKme added bug Something isn't working and removed enhancement New feature or request labels Feb 1, 2024
@ShGKme ShGKme modified the milestones: 8.6.1, 8.6.2, 8.6.3 Feb 1, 2024
@susnux susnux modified the milestones: 8.6.3, 8.7.0, 8.8.0 Feb 18, 2024
@Antreesy Antreesy modified the milestones: 8.8.0, 8.8.2 Feb 29, 2024
@susnux susnux marked this pull request as draft August 30, 2024 12:06
@GVodyanov
Copy link
Contributor Author

Fixed in #6054

@GVodyanov GVodyanov closed this Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working design Design, UX, interface and interaction design feature: app-navigation Related to the app-navigation component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove margin-right of mailbox counter if it has sub mailboxes
5 participants