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

FEAT: added Sorting dialog to the timer tab #67

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ronniedroid
Copy link

What is it?

  • Bugfix
  • Feature
  • Codebase improvement

Description of the changes in your PR

  • Added a dialog for sorting timers by creation time or timer duration
  • added config entries for timer sorting order
  • updated the menu accordingly
  • Update strings

Before/After Screenshots/Screen Record

  • Before:
  • After:

Fixes the following issue(s)

Acknowledgement

@ronniedroid ronniedroid changed the title Added sorting dialog to the timer tab FEAT: Added Sorting dialog to the timer tab Apr 21, 2024
@ronniedroid ronniedroid changed the title FEAT: Added Sorting dialog to the timer tab FEAT: added Sorting dialog to the timer tab Apr 21, 2024
@Aga-C
Copy link
Member

Aga-C commented Apr 22, 2024

Compare your solution to what's already in Alarms, because it behaves a bit differently:

  1. If you have many timers, that they don't fit on screen, sorting scrolls view to the bottom. It doesn't happen in Alarms.
  2. There's an animation with timers moving around. In Alarms there's no animation.

@ronniedroid
Copy link
Author

My PR did not add those, they were already there, I just added sorting.

@Aga-C
Copy link
Member

Aga-C commented Apr 22, 2024

But there was no sorting before, so no one could see this behavior. I know you've just added sorting, but while adding a new feature, we should make sure that it behaves the same as in other places.

@ronniedroid
Copy link
Author

But there was no sorting before, so no one could see this behavior. I know you've just added sorting, but while adding a new feature, we should make sure that it behaves the same as in other places.

Should I add animation to the alarms or remove it from the timers?

@Aga-C
Copy link
Member

Aga-C commented Apr 22, 2024

Don't add animation and remove it from Timers. We don't have animations while sorting in other apps.

@ronniedroid
Copy link
Author

@Aga-C I managed to remove the animation, but the auto scrolling I haven't figured out yet, I will try again later, but though I will say it here and maybe someone can share a tip or an idea.

@ronniedroid
Copy link
Author

I move to MyRecyclerViewAdapter for the TimerAdapter to be consistent with the AlarmsAdapter, no visual changes, but now we have no animations and the list does not scroll to the bottom.

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.

[Feature request] Add Sorting option - auto-sorting timers by time + set it as default option
2 participants