qw123wh / new-clock

The best clock app there is
Apache License 2.0
79 stars 27 forks source link

Bedtime mode #36

Closed Nilsu11 closed 1 year ago

Nilsu11 commented 1 year ago

I haven't figured out how to request the added permissions. Anyway you have to allow do not disturb access and set wallpaper. I also left many todos which should be resolved quickly(mostly it's ui stuff).

qw123wh commented 1 year ago

Hi thanks for your job, i tried the changes, if you go to modify or delete the "bedtime label", the app crashes. I think it's better to remove the "bedtime label" and maybe try pinning it to the top like in google clock.

As for the permissions, you have to give them in deskclock.java, like this as an example:

https://github.com/qw123wh/new-clock/commit/be4ab51c6192c02fe9ed7a01fbed64c91318cdf1

https://github.com/sanpei3/new-clock/commit/687f9ee97f7e4b68c46512ac1caed47260f47099

I had removed the phone permission, however android 13 requires it. I can eliminate it by following lineageos commits, but by doing so I reduce the compatibility of the app to old phones, compatibility would only be from android 8 and above. But this is another story

Nilsu11 commented 1 year ago

@qw123wh do you mean the bedtime label in the fragment layout or in the bottomsheet? both look pretty similar to me to googles version.

qw123wh commented 1 year ago

@Nilsu11 I mean this label highlighted in red, if you try to change or delete this label the app crashes. It would be better to remove this label and set “bedtime” at the top 41D26E46-A62D-4930-9100-11E40866B0F8 D5729AF1-4203-4266-AF79-B6CF940D5C44

Nilsu11 commented 1 year ago

@qw123wh I can't really exclude and move the alarm to the top so I decided to change the label to a string and hide the delete button. Is this good enough?

I can't resolve an error in the wake-up bottomsheet (changing the ringtone doesn't show until you switch to the alarm tab and then to the bedtime tab). Do you know anyone who can help me with this error?

qw123wh commented 1 year ago

@Nilsu11 The label now looks better

I don't know how to solve the ringtone problem, you could try to follow the timer settings and adapt it to bedtime.Basically i think you need to create a separate ringtone selection, in fact we have 2 separate classes which are Alarm and Timer

I see you added code for "ACCESS_NOTIFICATION_POLICY" but the permission needed for Android 13 is “ POST_NOTIFICATIONS” only this permission is not yet asked and you still need to enable notifications from the settings

qw123wh commented 1 year ago

@Nilsu11 I found another problem, if you try to delete bedtime from the alarm section, the app crashes. I think the bedtime section should only appear in the alarm section if it is turned on.

Nilsu11 commented 1 year ago

@qw123wh Yes post notification permission still is needed but access notification policy is needed for do not disturb access. In my next push I will request do not disturb access properly and the alarm will be moved to the top (but not in a top section), also it's label will be set to bold.

To your problem with deleting the alarm I also think it only should be shown if it is activated, but I don't have any idea on how to do this. I'm currently trying to prevent deleting it.

qw123wh commented 1 year ago

@Nilsu11 if it helps you this is the deleted bedtime log

https://paste.crdroid.net/PsM8gE

maybe you can solve it without forcing the "non-deletion"

Nilsu11 commented 1 year ago

@qw123wh would it be ok if the wake up alarm would be the next upcoming alarm?

qw123wh commented 1 year ago

@Nilsu11 I don't know I can't imagine it. In my opinion the important thing is to be able to delete it without errors. Surely there will be some users who will not use that function and will want to eliminate it from the alarm section.

Nilsu11 commented 1 year ago

@qw123wh This solution might not be perfect but now you you can delete the alarm.

qw123wh commented 1 year ago

@Nilsu11 Thank you for your valuable work that you are doing to bring this feature.

I think it needs to improve something more before the feature is finally merged.

even if “wake up” is deactivated the “bedtime” function should still work, they should be two separate things.

I send you a screen recorder of the errors that are still there.

https://paste.crdroid.net/Yiqq11

https://github.com/qw123wh/new-clock/assets/59417474/b47a1e75-7883-4b5d-9f91-a83e6a69c409

https://github.com/qw123wh/new-clock/assets/59417474/21733bfa-fc21-4634-a074-fa35f57e9456

https://github.com/qw123wh/new-clock/assets/59417474/8d214090-6b03-4c11-b9f4-a3dd2b9f9404

Another problem is that you disable "bed time" from notification, pause or turn off, the do not disturb mode remains active.

If I then pause it from the notification there is no possibility to always "resume" from the notification

Nilsu11 commented 1 year ago

@qw123wh I can't reproduce the error of do not disturb not deactivating.

I don't know what you mean with this: "If I then pause it from the notification there is no possibility to always "resume" from the notification" I implemented a resume now button

qw123wh commented 1 year ago

@Nilsu11 perfect, now it remains only to adjust the color, however I will merge it