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

refactor(mobile): refactor theme management #14415

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

dvbthien
Copy link
Contributor

@dvbthien dvbthien commented Nov 30, 2024

This pull request refactors the theme management in the mobile application. The changes include:

  • Splitting and refactoring theme-related files: The theme-related code has been reorganized for better maintainability and readability.
  • Updating theme providers: Modified the theme providers to handle the color scheme and dynamic theming in Android.

I have tested it on both IOS and Android
How to test it: Change the theme and use dynamic theme in Android. Both should work as expected.

Next stage:
Hi @alextran1502

For the next pull request, I plan to declare common colors, font sizes, styles, paddings, and borders to ensure a consistent interface throughout the app.

Additionally, I've found two font options: Manrope and InterTight. I think they both look good. I'll post images of both fonts for your review. If you prefer one over the other, please let me know. I'll apply the chosen font to both the web and mobile app.

Here's a link to the image of the font: https://imgur.com/a/c5QAib0

What are your thoughts?

@alextran1502
Copy link
Contributor

Thank you for the PR! Yes, we should standardize on a UI for form/card for consistency.

I haven't found a font that expresses the App UI besides Overpass yet. And I have tried a few. I will return to the font topic another time.

@dvbthien
Copy link
Contributor Author

dvbthien commented Dec 2, 2024

Thanks for your reply. I will create a pull request for common colors, font sizes, styles, paddings, and borders. I plan to refactor it step by step.

Feels happy to contribute to this project.

const ImmichColorPreset defaultColorPreset = ImmichColorPreset.indigo;
const String defaultColorPresetName = "indigo";

const Color immichBrandColorLight = Color(0xFF4150AF);
Copy link
Member

@shenlong-tanwen shenlong-tanwen Dec 2, 2024

Choose a reason for hiding this comment

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

Since you're working on unifying colours and theme across the app, I'd like the global variables for colours to be removed from the codebase as well. Maybe if not in this PR, Kindly consider this in your upcoming PRs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @shenlong-tanwen,

Thanks for your suggestion. Can you explain it to me in more detail? If you have any ideas for unifying colors and themes across the app, let me know. I'll implement them in upcoming PRs. I just started coding in Flutter, so your ideas will help me a lot.

@jrasm91
Copy link
Contributor

jrasm91 commented Dec 3, 2024

A lot of these changes are related to const/linting rule. Can you make all those changes in a separate PR?

@dvbthien
Copy link
Contributor Author

dvbthien commented Dec 3, 2024

A lot of these changes are related to const/linting rule. Can you make all those changes in a separate PR?

Sure, I handled that in the last commit. Thanks for the suggestion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants