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 beta light/dark mode #119

Closed
wants to merge 3 commits into from

Conversation

giovanni-ercolano
Copy link
Contributor

I've added the appearance change within the app, unfortunately at the moment it doesn't render as well as it could but this is due to some pages not being complete. That said at the moment the choice between light or dark is not saved this is why I wanted to ask how to proceed, I was thinking of using shared_preferences to save locally.

@theperu
Copy link
Collaborator

theperu commented Oct 18, 2023

Good work! The color scheme seems ok but I noticed some minor issues here and there:

  1. In the dashboard the account names and amounts are white which makes them really hard to read
    Screenshot_2023-10-18-17-05-05-486_com example sossoldi

  2. In the settings the icons are dark black and blue while in the dashboard are white and darkblue
    Screenshot_2023-10-18-17-05-17-836_com example sossoldi

  3. Under the dark mode icon I see the icon for the currency in white (also it might not be centered)
    Screenshot_2023-10-18-17-05-21-622_com example sossoldi

  4. In the "New transaction" page the name for the account and the date might be a little bit hard to read since they are dark grey
    Screenshot_2023-10-18-17-06-23-859_com example sossoldi

  5. The line graph in the dashboard and graphs page need to be adapted to support also the dark mode

As you can see this are all pretty small things that we can clarify also with the help of @federicopozzato but overall as I already said I think it looks fantastic!

@federicopozzato
Copy link
Contributor

Hi guys! Thank you for reaching out to me for this issue.

For the icons it may be needed a dark version of them with the white color as a main one. I can provide them in the figma in the dark mode section and then you can export them.

For the texts.... yes you're right, thank you for pointing out those problems! I think that if you can override the color and use a dark one for when the contrast is not enough, that would be awesome. On my side, i can provide a dark version for the categories and accounts color (it will be the same hue as the ligh mode, but with a bit less brightess).

I'll wait for a confirmation of yours to preceed with the things i mentioned.

Thank youuuu

@theperu
Copy link
Collaborator

theperu commented Oct 18, 2023

Sounds good to me, thanks! You can post any update here then 😁

@giovanni-ercolano
Copy link
Contributor Author

Thank you for pointing out these small problems, I will fix them in time. As for the icons, I await updates regarding a possible dark version. Thank you all for your help and availability, this is my first contribution to an open source project, I am glad you are enjoying it. For any other updates feel free to contact me or write here in the comments.

@federicopozzato
Copy link
Contributor

So I've cheched better the icon issue and I've noticed that the one currently used are an old version of the ones that there are on figma: the icons are now fill with a white color, so they look good also in the dark mode. You can find both kind of icons (light and dark version) in the "components" frame in figma.

As for the colors, I've updated all the palettes' frames to help you guys understand them better, and I've added the dark mode version of the accounts and categories colors.

The icons for the dark mode switch and the currency selections are not in figma because that "general settings" page hasn't been designed yet, but I suggest to use a switch for the dark/light mode, and a selector for the currency.

@giovanni-ercolano
Copy link
Contributor Author

giovanni-ercolano commented Oct 19, 2023

Perfect thank you very much @federicopozzato , I have already adopted the changes you made, I in the meantime have already fixed points 1, 2 and 3 mentioned by @theperu , as soon as I finish these days I will show you the result.

As for the general settings page for now I am leaving the icon style like this until it is decided how to proceed.

@mikev-cw
Copy link
Collaborator

mikev-cw commented Nov 9, 2023

Hi, thank you for this contribution, very good approach!
Beyond what already been said, I'm adding a couple more minor issues:
To keep a consistent coding style, we use relative paths when importing files from within our app.

Before:
import 'package:sossoldi/model/example.dart';

After:
import 'model/example.dart';

As long as we use it consistently, any of the two styles will have the same performance. However, it's important not to mix them as that might lead to some issues. So please check you imports.

Also, we would greatly appreciate if you could remove all files outside /lib folder from this request, as not pertinent and related to your specific system.

Thank you again!!

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.

4 participants