-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
feat(mobile): new settings ui #19211
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
base: main
Are you sure you want to change the base?
Conversation
Hi Thien,
Looks good, when I go through the final tests, I can make small adjustments if needed |
Hi @alextran1502, I have made the changes per your suggestions. I have also added a header titled "Placeholder" for all sections. Please review it when you have time. Additionally, I have a new design for the backup page. Please see the image below. ![]() |
…immich into mobile/new-settings-ui
Hi @alextran1502, Everything has been completed for you to review in the last two commits. If you find it difficult to review, I can revert those changes and create a separate pull request.
Have a nice day, |
Hi @dvbthien, please keep this PR containing the new design for the settings stuff. Other refactoring should be in their PRs. Regarding the fonts, I would like to make this change when needed. It is an important factor in the whole app's look and feel |
Hi @alextran1502, I have reverted the commits. Please review them when you have a moment. Regarding the Mulish font, I find it very similar to Overpass. I think it's beautiful, comfortable, and easy to read. You can try it out when you have time Thanks |
@@ -0,0 +1,78 @@ | |||
import 'package:flutter/material.dart'; | |||
|
|||
class FadeOnTap extends StatelessWidget { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this for? I don't notice the different of the components that use this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can long-press to notice the difference—using onTap
in ListTile
triggers the default splash effect. I used FadeOnTap
to avoid the splash while still providing tap feedback. The default splash effect can be tricky to handle in some areas due to padding or borders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
11.mp4
22.mp4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we want to remove the default splash behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used padding in SettingsCardLayout
to align all items and set the ListTile
padding to 0. With the default splash behavior, the ripple ignores that padding (as shown below) and can’t be properly clipped where there’s a border. I also chose a Container
over a Card
for the settings card to save on performance, since we don’t need elevation—but the container doesn’t clip the splash as cleanly as a card does.
Description
Redesigned settings pages with a more modern and eye-catching interface.
I have attached images in the Screenshots
Screenshots (if appropriate)
Checklist: