chore: Resolve #356 - Reworked currency selector in General Settings

[bongio94]

🎯 Description

This PR reworks the currency selector that can be found in Settings -> General Settings. The old currency selector had some issues with the colors of the ListTiles and, taking in consideration the discussion that is happening in #333 about adding the system theme as option, I went with the updated design.

Visual implementation follows this Figma frame.

Diving a bit deeper into the changes:

• Added onSurfaceVariant to customColorScheme and darkCustomColorScheme to have a lighter color that gives enough contrast when used on surface (darkBlue6). Please check these messages ([1], [2]) for more details on this addition.

• In lib\pages\general_options\widgets I've added the selector folder. It contains two files, selector_container.dart and selector_tile.dart:

  • SelectorContainer is a simple Widget responsible for the background and the top label. It takes a label and a child.
  • SelectorTile is the list tile like UI element which shows the currencies list (and ideally should also be used to show the appearance options).
    I've tried to make it flexible while still maintaining the convenience of use. The important part for me here was to have a widget that can give a uniform look to the settings page and that can be reused when more settings will be added to the page. I know it will need to be updated as requirements will change but I can take care of that when time comes.
    It has a customisable title, trailing (String) and trailingWidget. This widget is meant to handle selection between a number of options, for this purpose the isSelected bool can be used. Also added a void onTap.

• The old CurrencySelectorDialog has been renamed to SettingsCurrencySelector and it uses SelectorContainer and SelectorTile while maintaining the same currency selection state handling that was used in the previous implementation.

• SettingsCurrencySelector is then used in lib\pages\general_options\general_settings.dart.

• For what concerns testing:

  • Added some simple tests for the SelectorTile basic interactions.
  • Added a convenience extension on WidgetTester in test\test_utils. It's called pumpWidgetWithMaterialApp and as the name suggests, it's just a wrapper on pumpWidget which includes a MaterialApp and the Material widget. I've added an option to toggle on or off the ProviderScope as well (defaults to on).

Please check the latest additions here.

Closes: #356

📱 Changes

• [X] Describe key changes made
• [X] List any new features, bug fixes, or refactors
• [X] Include additional details if necessary

🧪 Testing Instructions

Behaviour

1. Go to Settings -> General Settings
2. Change your selected currency

📸 Screenshots / Screen Recordings (if applicable)

Before	After
https://github.com/user-attachments/assets/e949b3cc-c240-46d1-a46f-5a9b345f1f3d	https://github.com/user-attachments/assets/b4ac15cb-11c7-43df-9047-16cc669f5645

🔍 Checklist for reviewers

• [X] Code is formatted correctly
• [X] Tests are passing
• [X] New tests are added (if needed)
• [X] Style matches the figma/designer requests
• Tested on:
  • [ ] iOS
  • [X] Android

✍️ Additional Context

I'd appreciate if someone could test this on iOS but I'm not expecting any change 🙂

Note that this new UI might look a bit out of place because the appearance switcher is still the same. Hopefully when both components use this new layout it'll look better as a whole 😅

[fres-sudo]

I'll try to review this, and test on iOS, by tomorrow evening or sunday at max, thanks for the PR!

[bongio94]

Regarding this comment I made:

Note that this new UI might look a bit out of place because the appearance switcher is still the same. Hopefully when both components use this new layout it'll look better as a whole 😅

I think this could be improved a bit further and make it ready for future settings by making the SelectorContainer an expandable tile.

Here's a quick demo;

https://github.com/user-attachments/assets/a649a067-4e35-4c05-bce5-a1a430d83d15

[fres-sudo]

I think this could be improved a bit further and make it ready for future settings by making the SelectorContainer an expandable tile.

I think it's a very cool design the one in the demo, why not implement it? Since we actually introduced 3 theme modes in #333?

[bongio94]

Hey @fres-sudo

Thanks for the detailed review 🙌 I'll start to go through your comments now 🙂

[bongio94]

I think this could be improved a bit further and make it ready for future settings by making the SelectorContainer an expandable tile.

I think it's a very cool design the one in the demo, why not implement it? Since we actually introduced 3 theme modes in #333?

I would want to wait for @federicopozzato to approve this first. While I think it's in line with the Figma specs, it's still a bit different.

[bongio94]

Thanks for the PR!

I tested on iOS, it works just fine, the only small issue that I found is that ambigous spacing:

You can easily spot it under the last element of the list...is it intended? bcuz I can't see it in your last video demo..

hmm, I don't think that's the separatorBuilder from ListView.separated, as you can see it's not showing in the original screenshot attached.

Let's wait for the new layout to be pushed and you can re-test

[fres-sudo]

Let's wait for the new layout to be pushed and you can re-test

Wdym by the "new layout" what did I miss?

[bongio94]

Let's wait for the new layout to be pushed and you can re-test

Wdym by the "new layout" what did I miss?

Wrong word probably as the layout is the same 😅 I meant the new interaction, with the expansion functionality

[federicopozzato]

Guys, we appreciate you effort and propositivity, but it's always a metter of process. Things has to be DESIGNED FIRST, then developed. I can't stress this enough. Multicurrency is something we have to work on for sure, but I'm not sure that here will be just a currency selector as a feature: what if I want to have multiple currencies at the same time? @theperu I don't remember what we said about it for the 1.0

[theperu]

@federicopozzato this is not an implementation of the multi currency feature. This was opened to align the graphic of the theme selector that has changed in #333 so that we don't have two completely different styles between the two elements

[bongio94]

@theperu @federicopozzato Should I move forward with this? #333 is dependent on this piece of work.

The recording shows how everything should look when both the theme selector and the currency selector adopts the new UI https://github.com/RIP-Comm/sossoldi/pull/360#issuecomment-2745141974

[theperu]

Ok for me but I'll leave the last word to @federicopozzato

[federicopozzato]

@bongio94 thanks for the details. Yes go on

[bongio94]

PR details have changes a bit after the latest commits, I'll try to go through the latest changes with this comment and I'll link this to the PR description.

After @fres-sudo suggestions:

• I've removed all of the Gap widgets (gap dependency has been removed).
• Added asserts on SelectorTile constructor for ensuring that only one between trailing and trailingWidget is used.
  I've also added an optional trailingTextStyle which gives the ability of customizing the trailing text TextStyle.
• Added the expansion behaviour. In particular:
  • SelectorContainer now has an initiallyExpanded property, which controls the initial expansion state (whether or not the container should be expanded on instantiation), and a hasTrailingChevron property which toggles on or off the trailing chevron which rotates when the container expands or collapses.
  • The label changes basing on the expansion status. When collapsed the label is bigger (fontSize 14, w600), when expanded is smaller (fontSize 12, w400)

[bongio94]

@fres-sudo Could you have a look at my comment here and close the change request if it sounds good?

[theperu]

Hi @bongio94👋 We recently merged a PR that changed the folder structure, which caused some conflicts here. When you get a chance, could you take a look and resolve them?

If there’s no response in the next 15 days, we’ll consider this PR inactive and plan to close it to keep things tidy.

Thanks!