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

Added UI Settings for top bar font weight #63

Merged
merged 5 commits into from
Sep 28, 2023

Conversation

bhavesh100
Copy link
Contributor

No description provided.

Copy link
Owner

@lorenzovngl lorenzovngl left a comment

Choose a reason for hiding this comment

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

I built and ran the app with these changes and... thinking about it... a separate activity for UI settings is not the best idea considering the limited number of settings.

I think the best choice is to put all in the SettingsActivity, separating behavioral settings from appearance settings using two headers:

Behavior
[Date format]
[Notification time]
Appearance
[Theme]
[Dynamic colors]
[Top app bar font weight]

Copy link
Owner

@lorenzovngl lorenzovngl left a comment

Choose a reason for hiding this comment

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

Perfect, only one thing left. There is this little bug when you switch between the options, but I think you can fix it easily.

Registrazione.2023-09-28.110250.mp4

@bhavesh100
Copy link
Contributor Author

I don't know why this bug is coming. I just interchanged the if conditions the bug went away.

Copy link
Owner

@lorenzovngl lorenzovngl left a comment

Choose a reason for hiding this comment

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

Looks awesome, thank you!

@lorenzovngl lorenzovngl merged commit 4caa2fc into lorenzovngl:main Sep 28, 2023
1 check failed
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.

None yet

2 participants