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.

Sign up for GitHub

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

Jump to bottom

Overhauls the settings screen #3140

Merged
merged 8 commits into from
Feb 1, 2022
Merged

Overhauls the settings screen #3140

merged 8 commits into from
Feb 1, 2022

Conversation

nicbn
Copy link
Contributor

@nicbn nicbn commented Jan 22, 2022 •
edited
Loading

Description

Video demonstration: https://www.youtube.com/watch?v=xo8mEtx8GCg

Issue tracker

Closes #2968

Manual tests

  • Done

Tested on API 30 and API 16

Build tasks success

Successfully running following tasks on local:

  • ./gradlew assembledebug
  • ./gradlew spotlessCheck

@EmmanuelMess
Copy link
Member

EmmanuelMess commented Jan 22, 2022 •
edited
Loading

Do not move stuff to utils, that name is meaningless.

@EmmanuelMess
Copy link
Member

Please beware of tests that might fail after the name changes.

@nicbn
Copy link
Contributor Author

nicbn commented Jan 22, 2022 •
edited
Loading

Should I move the constants to the base package? I don't think it makes sense to have these constants at the fragments package, as most of their usage doesn't concern fragments, and is related to SharedPreferences keys instead.

Or should I revert these changes and PR a refactor separately later?

@EmmanuelMess
Copy link
Member

Should I move the constants to the base package?

No.

I don't think it makes sense to have these constants at the fragments package, as most of their usage doesn't concern fragments, and is related to SharedPreferences keys instead.

I would move it to where most sense it makes for to be at, but I am not quite sure that place exists here, feel free to create one if needed.

Or should I revert these changes and PR a refactor separately later?

As you wish.

@nicbn nicbn marked this pull request as ready for review January 26, 2022 15:02
EmmanuelMess
EmmanuelMess requested changes Jan 26, 2022
View reviewed changes
Copy link
Member

@EmmanuelMess EmmanuelMess left a comment

Choose a reason for hiding this comment

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

Needs a few changes.

app/src/main/java/com/amaze/filemanager/ui/activities/PreferencesActivity.kt Outdated Show resolved Hide resolved
app/src/main/java/com/amaze/filemanager/ui/activities/PreferencesActivity.kt Outdated Show resolved Hide resolved
app/src/main/java/com/amaze/filemanager/ui/activities/PreferencesActivity.kt Outdated Show resolved Hide resolved
app/src/main/java/com/amaze/filemanager/ui/dialogs/ColorPickerDialog.java Outdated Show resolved Hide resolved
app/src/main/java/com/amaze/filemanager/ui/dialogs/ColorPickerDialog.java Outdated Show resolved Hide resolved
...n/java/com/amaze/filemanager/ui/fragments/preference_fragments/QuickAccessesPrefsFragment.kt Outdated Show resolved Hide resolved
...c/main/java/com/amaze/filemanager/ui/fragments/preference_fragments/SecurityPrefsFragment.kt Outdated Show resolved Hide resolved
app/src/main/java/com/amaze/filemanager/ui/fragments/preference_fragments/UiPrefsFragment.kt Outdated Show resolved Hide resolved
app/src/main/java/com/amaze/filemanager/ui/fragments/preference_fragments/UiPrefsFragment.kt Show resolved Hide resolved
app/src/main/java/com/amaze/filemanager/ui/views/preference/PathSwitchPreference.kt Outdated Show resolved Hide resolved
@EmmanuelMess
Copy link
Member

There is a detail, why does it take so long between the click and the opening of the section on my Nokia? It should be almost instant, but it takes a few frames, the "touch" animation almost ends before the screen opens.

@nicbn
Copy link
Contributor Author

nicbn commented Jan 26, 2022

That's because of the transition I was using. Changed it to a fade transition which feels way more responsive.

This is not what is used in Android settings, which uses the "open" transition, but I found it a bit distracting.

@EmmanuelMess
Copy link
Member

That's because of the transition I was using. Changed it to a fade transition which feels way more responsive.

This is not what is used in Android settings, which uses the "open" transition, but I found it a bit distracting.

Its better now.

EmmanuelMess
EmmanuelMess previously approved these changes Jan 27, 2022
View reviewed changes
Copy link
Member

@EmmanuelMess EmmanuelMess left a comment

Choose a reason for hiding this comment

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

Thanks!

@EmmanuelMess
Copy link
Member

@nicbn sorry for the delay, Codacy had an issue with their services and the Codacy status wasn't getting through to the PR, could you fix the Codacy issues so we can merge please?

@nicbn
Copy link
Contributor Author

nicbn commented Jan 28, 2022

Done

EmmanuelMess
EmmanuelMess previously approved these changes Jan 28, 2022
View reviewed changes
VishnuSanal
VishnuSanal requested changes Jan 31, 2022
View reviewed changes
Copy link
Member

@VishnuSanal VishnuSanal left a comment

Choose a reason for hiding this comment

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

@nicbn Excellent work!

  • Please address the comments
  • Also, where are the new icons from? Did you create them on your own or imported? If they're imported, please link us to the source.

app/src/main/java/com/amaze/filemanager/ui/activities/superclasses/PreferenceActivity.java Show resolved Hide resolved
app/src/main/java/com/amaze/filemanager/ui/activities/superclasses/ThemedActivity.java Show resolved Hide resolved
app/src/main/java/com/amaze/filemanager/ui/fragments/preference_fragments/BasePrefsFragment.kt Outdated Show resolved Hide resolved
@nicbn
Copy link
Contributor Author

nicbn commented Jan 31, 2022

Also, where are the new icons from? Did you create them on your own or imported? If they're imported, please link us to the source.

They are clip arts built into Android Studio, IIRC they are from https://fonts.google.com/icons

@nicbn nicbn marked this pull request as draft January 31, 2022 16:51
@nicbn nicbn marked this pull request as ready for review February 1, 2022 14:18
@VishalNehra
Copy link
Member

What happen to 'send feedback' option? It used to option email app prior to this. Other changes seem really good.

@nicbn
Copy link
Contributor Author

nicbn commented Feb 1, 2022 •
edited
Loading

It seems to be working the same in my device.

It asks what app to open, and if you select the email app then it will fill it, both before and after this PR.

@EmmanuelMess EmmanuelMess merged commit 74e8e68 into TeamAmaze:release/3.7 Feb 1, 2022
@EmmanuelMess
Copy link
Member

Here are some other bounties.

@VishnuSanal VishnuSanal mentioned this pull request Feb 6, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VishnuSanal VishnuSanal VishnuSanal approved these changes

@EmmanuelMess EmmanuelMess EmmanuelMess approved these changes

@TranceLove TranceLove Awaiting requested review from TranceLove

@VishalNehra VishalNehra Awaiting requested review from VishalNehra

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add different panes to preferences
4 participants
@nicbn @EmmanuelMess @VishalNehra @VishnuSanal