Skip to content

Implement account menu design#564

Merged
n13 merged 3 commits into
mainfrom
fix-account-menu
Jul 14, 2026
Merged

Implement account menu design#564
n13 merged 3 commits into
mainfrom
fix-account-menu

Conversation

@n13

@n13 n13 commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Implement account menu designs
Simulator Screenshot - iPhone 17 - 2026-07-14 at 12 51 50
Simulator Screenshot - iPhone 17 - 2026-07-14 at 12 51 54

title: l10n.addAccountMenuImportKeystoneTitle,
subtitle: l10n.addAccountMenuImportKeystoneSubtitle,
onTap: _onImportKeystone,
colors: colors,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Since this widget is a proper widget instead of a var or just method. I think we can get the colors and text inside the widget definition instead of passing as parameters. This way we don't repeat unnecessarily.

icon: Icons.radar_outlined,
title: l10n.addAccountMenuDiscoverMultisigTitle,
subtitle: l10n.addAccountMenuDiscoverMultisigSubtitle,
colors: colors,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same issue here with repeating colors and text theme.

child: Row(
mainAxisAlignment: MainAxisAlignment.spaceBetween,
children: [
Text(title, style: text.paragraph?.copyWith(fontSize: 18, color: colors.textPrimary)),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

AI really like to do unnecessary override like colors.textPrimary.

We don't need to, text primary is the default color.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

theyre not uneccessary

@dewabisma dewabisma left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM! Just few nits.

@n13
n13 merged commit 309d252 into main Jul 14, 2026
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.

2 participants