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

Eckhart vertical menu improvement #4617

Open
wants to merge 15 commits into
base: ui-eckhart
Choose a base branch
from

Conversation

bieleluk
Copy link
Contributor

This PR:

  • creates a new button content type MenuItem with title and optional subtitle
  • creates 3 new button stylesheets for menu item title and one for menu item subtitle
  • adds 2 new battery-related icons
  • splits Vertical menu component into two full-screen and partial components
  • makes vertical menu scrollable and preserves the touch functionality in the menu

@bieleluk bieleluk added the T3W1 label Feb 12, 2025
@bieleluk bieleluk requested a review from obrusvit February 12, 2025 12:33
@bieleluk bieleluk requested a review from prusnak as a code owner February 12, 2025 12:33
@bieleluk bieleluk removed the request for review from prusnak February 12, 2025 12:33
@bieleluk bieleluk changed the title Cckhart vertical menu improvement Eckhart vertical menu improvement Feb 12, 2025
Copy link

github-actions bot commented Feb 12, 2025

core UI changes device test click test persistence test
T2T1 Model T test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3B1 Safe 3 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3T1 Safe 5 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
All main(screens)

Copy link
Contributor

@obrusvit obrusvit left a comment

Choose a reason for hiding this comment

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

A few comments to discuss.

core/embed/rust/src/ui/layout_eckhart/theme/mod.rs Outdated Show resolved Hide resolved
@@ -37,6 +37,8 @@ pub struct Button {

impl Button {
pub const BASELINE_OFFSET: Offset = Offset::new(2, 6);
pub const MENU_Y_BASE_OFFSET: i16 = 28;
pub const MENU_LINE_SPACING: i16 = 7;
Copy link
Contributor

Choose a reason for hiding this comment

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

These constants probably don't need to be pub

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's do the Button::height() implementation then, as discussed today.

core/embed/rust/src/ui/layout_eckhart/component/button.rs Outdated Show resolved Hide resolved
core/embed/rust/src/ui/layout_eckhart/component/button.rs Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 🔎 Needs review
Development

Successfully merging this pull request may close these issues.

2 participants