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/multiple timers #285

Merged
merged 35 commits into from
Sep 12, 2021
Merged

Conversation

KryptKode
Copy link
Contributor

@KryptKode KryptKode commented Sep 2, 2021

Notes

  • add support for multiple timers
  • separate timer actions from states by adding a new class TimerEvent to hold actions Reset, Start, Pause, Finish, Refesh that could be performed on a timer.
  • make windowSoftInputMode=adJustPan for MainActivity
  • should address this request

Screenshot

screencapture-1631056793013.mov

- feat: separate timer actions from states by adding a new class TimerEvent to hold actions Reset, Start, Pause, Finish, Refesh that could be performed on a timer.
- feat: handle multiple countdown timers in the App file by creating a map of the timer id to the countdown timer.
- fix: use gson instance from  TypeAdapter in Room's Converters class
- ref: remove scroll view parent from each timer item, a fix for the keyboard obscuring a label will be implemented in a future commit
- add page indicator
- add default timer based on saved prefs when the database is created
- post scolling to the first timer when a new timer is added
- this ensures the textfield is always above the keyboard
@tibbi
Copy link
Member

tibbi commented Sep 3, 2021

we have an ic_plus_vector at Simple Commons, use that instead of the newly created ic_add_vector

@tibbi
Copy link
Member

tibbi commented Sep 3, 2021

I would prefer a more compact layout instead of the vertical viewpager, something like at the alarms at the second tab. One where the user could see the labels and timers easily and use them quickly. The current solution could be difficult to manage if there are more than 10 timers.

@tibbi
Copy link
Member

tibbi commented Sep 3, 2021

some text colors and buttons are not updating colors properly, try using light theme for testing too

@KryptKode
Copy link
Contributor Author

I would make the changes

@tibbi
Copy link
Member

tibbi commented Sep 3, 2021

alright, thanks. I know that the UI changes might need some time and the viewpager you added wont be used, but I sometimes dont know how do I want things to look. I only figure it out once I test one alternative and see that I prefer the other one :)

@KryptKode
Copy link
Contributor Author

alright, thanks. I know that the UI changes might need some time and the viewpager you added wont be used, but I sometimes dont know how do I want things to look. I only figure it out once I test one alternative and see that I prefer the other one :)

No problem. I wanted to preserve the original look and feel. With this change, each item would have the Play/Pause, Reset and Delete buttons. Then a general FAB for adding on the left.

@tibbi
Copy link
Member

tibbi commented Sep 3, 2021

ye it would, but it is hard to manage once you have multiple items

@KryptKode
Copy link
Contributor Author

@tibbi I made the changes. Please take a look

@tibbi
Copy link
Member

tibbi commented Sep 6, 2021

I didnt mean just copying the same Timer UI multiple times vertically, try compressing it like the Alarm view too. At least 6-7 timers should fit onto 1 screen, they should be split with some margin to be able to differentiate them well

@tibbi
Copy link
Member

tibbi commented Sep 6, 2021

show the curent timer and label at the main tab, options like vibrating and ringtone should be shown only after clicking. Somewhat like at the alarms. Deleting should be done with long pressing the timer item, like at alarms too.

- implement EditTimerDialog
- adding a new timer would show the dialog
- clicking on a timer item would show the dialog
@KryptKode
Copy link
Contributor Author

@tibbi Please check for the play/pause button,


Without highlighted colour With Highlighted colour

@KryptKode
Copy link
Contributor Author

@tibbi Also, should clicking on the time text give the option to adjust the time or should it be done solely from the edit dialog?
If we allow changing the time by clicking the time text, it might interfere with the long press to select. Also, should there be a delete button for each item?

screencapture-1631048194584.mp4

@tibbi
Copy link
Member

tibbi commented Sep 7, 2021

first lets move the play/pause to the right side, with the timer and label being shown at the left. It will be more compact like that. Delete should be shown at the top menu only after long pressing some item. Regarding the time, make it work like at the Alarm tab. Clicking it should just open the edit dialog, like at clicking the label. Then you have to press the time again to change it.

@KryptKode
Copy link
Contributor Author

Made the changes. Please check. @tibbi

screencapture-1631127086547.mp4

@tibbi
Copy link
Member

tibbi commented Sep 10, 2021

nice, 2 more things.

1.If I launch a timer, background and foreground the app to make the top notification appear, then delete the timer, the notification doesnt disappear. It just stops the countdown.
2. colors are not handled properly, try switching to light theme for some testing

@KryptKode
Copy link
Contributor Author

@tibbi I made the changes

@tibbi
Copy link
Member

tibbi commented Sep 10, 2021

if you click on the time, the timer icon above Vibrate is still white. Just switch to light theme and change the background to very light grey to find such issues

@KryptKode
Copy link
Contributor Author

Seen, will fix.

@tibbi
Copy link
Member

tibbi commented Sep 10, 2021

maybe one more thing. If you change the text color, the Play and repeat button colors are not updated

@KryptKode
Copy link
Contributor Author

maybe one more thing. If you change the text color, the Play and repeat button colors are not updated

@tibbi Please what steps did you follow to detect this? I am unable to reproduce it

@tibbi
Copy link
Member

tibbi commented Sep 10, 2021

seems to occur only at paused timers, when the Repeat button is visible too. So basically toggle play/pause 2x, so you see both the repeat and Play button. Then go into the app settings, change the text color and those 2 buttons will still have the old color.

@KryptKode
Copy link
Contributor Author

KryptKode commented Sep 10, 2021

Still unable to reproduce after following the steps 😔

screencapture-1631294312406.mp4

@tibbi
Copy link
Member

tibbi commented Sep 10, 2021

do not change the whole theme, just the text color

val contextText = when {
runningTimers.size > 1 -> getString(R.string.timer_multiple_notification_msg, runningTimers.size)
firstTimer.label.isNotEmpty() -> getString(R.string.timer_single_notification_label_msg, firstTimer.label)
else -> getString(R.string.timer_single_notification_msg, runningTimers.size)
Copy link
Member

Choose a reason for hiding this comment

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

The above timer_multiple_notification_msg has to be added as a Plural in the strings file so it can be properly translated in case it is 2 timers or 5, as it differs in some languages. I mean like https://github.com/SimpleMobileTools/Simple-Commons/blob/7c0a8d628ee23094e669a6169220b8da0fe9e649/commons/src/main/res/values/strings.xml#L165 . Add the "one" usecase too, then you can remove the timer_single_notification_msg string

@tibbi
Copy link
Member

tibbi commented Sep 12, 2021

alright, looks like we are done now, thanks :)

@tibbi tibbi merged commit 8432684 into SimpleMobileTools:master Sep 12, 2021
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