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

Add constant proportion icons to Main-menu #1643

Merged

Conversation

ArturoManzoli
Copy link
Contributor

@ArturoManzoli ArturoManzoli commented Feb 3, 2025

Adds custom svg icons to the main menu, so they will keep constant proportions and will never be off-center again.

Icons are in a Figma template with the rest of the Cockpit mockups. When adding a new item on the main menu, just drag the mdi icon svg inside an empty circle, resize it and export as an svg.

image

Screenshare.-.2025-02-03.11_58_20.AM.mp4

Fix #1445

@rafaellehmkuhl
Copy link
Member

Should wait for #1595 to be merged.

@ArturoManzoli
Copy link
Contributor Author

Should wait for #1595 to be merged.

Agreed

Copy link
Contributor

@ES-Alexander ES-Alexander left a comment

Choose a reason for hiding this comment

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

  1. Icon consistency seems to work - nice! 😃
  2. For some reason the (taller) internal menu spreads out the text extra, and goes past the top and bottom, without the sidebar being scrollable
Screen.Recording.2025-02-04.at.4.27.29.am.mov

@rafaellehmkuhl rafaellehmkuhl mentioned this pull request Feb 3, 2025
@rafaellehmkuhl
Copy link
Member

rafaellehmkuhl commented Feb 3, 2025

  1. For some reason the (taller) internal menu spreads out the text extra, and goes past the top and bottom, without the sidebar being scrollable

#1595 should help with this as it's getting two of those submenus out of this menu, but I think the long term solution belongs to a further PR addressing #1645.

Both #1595 as well as this one fix other bugs that are currently on master and have similar or higher criticality, so I believe it would be better not to increase their scope right now.

Copy link
Contributor

@ES-Alexander ES-Alexander left a comment

Choose a reason for hiding this comment

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

Base bug is fixed, so let's get this in and fix the menu scaling stuff later :-)

@rafaellehmkuhl
Copy link
Member

@ArturoManzoli just need to rebase this over master and we can merge it.

Copy link
Member

@rafaellehmkuhl rafaellehmkuhl left a comment

Choose a reason for hiding this comment

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

Something in the new style is causing the menu to get a lot taller:

image image

@ArturoManzoli ArturoManzoli force-pushed the add-main-menu-custom-icons branch from 0334d9a to bb06bf5 Compare February 3, 2025 20:13
@ArturoManzoli
Copy link
Contributor Author

  1. Icon consistency seems to work - nice! 😃
  2. For some reason the (taller) internal menu spreads out the text extra, and goes past the top and bottom, without the sidebar being scrollable

Thanks for pointing that out. Some margin settings were missing.

@ArturoManzoli
Copy link
Contributor Author

Something in the new style is causing the menu to get a lot taller:

Yep, thanks for pointing that out! Its fixed now

Copy link
Member

@rafaellehmkuhl rafaellehmkuhl left a comment

Choose a reason for hiding this comment

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

The changes are breaking the current menu layout resizing logic.

Master:
image

This PR:
image

Copy link
Member

@rafaellehmkuhl rafaellehmkuhl left a comment

Choose a reason for hiding this comment

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

Everything seems to be working fine now!
Nice work!

@ArturoManzoli ArturoManzoli merged commit 1c64500 into bluerobotics:master Feb 4, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Main-menu icons get out of center on some window sizes
3 participants