Added Visual confirmation when Wake Up timer is set. Request solution #502. #554

robercoding · 2021-03-10T15:49:03Z

Changes

Implemented solution showing with a snackbar to user of when alarm will go of.
For that I have added different strings so it's more clear to the user.
Added unit tests tests making sure appropiate strings appear on screen.
Added UI Test checking snackbar shows up.

Testing

[✔️] Tested on a emulator device
[✔️] Added unit tests.
[✔️] Added UI test.

Others

Resolves Visual confirmation that the Wake Up timer is set #502

Tell me if you like it or want some specific changes!

…o get the correct string to notify user the schedule left time.

…orrect string.

codecov · 2021-03-10T15:56:58Z

Codecov Report

Merging #554 (4ce7197) into master (9f286c7) will decrease coverage by 0.61%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #554      +/-   ##
==========================================
- Coverage   49.09%   48.48%   -0.62%     
==========================================
  Files          32       32              
  Lines        1821     1844      +23     
  Branches      175      179       +4     
==========================================
  Hits          894      894              
- Misses        880      903      +23     
  Partials       47       47

Flag	Coverage Δ
android	`45.02% <0.00%> (-0.62%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...shutoshgngwr/noice/fragment/WakeUpTimerFragment.kt	`0.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1843337...4ce7197. Read the comment docs.

robercoding · 2021-03-10T16:06:39Z

I don't know why the commit 7e6fedf8 did that thing on strings.xml. I think it's because of different formating or something (I use Android Studio on Windows 10), but sorry about that!

ashutoshgngwr · 2021-03-10T16:09:53Z

I don't know why the commit 7e6fedf8 did that thing on strings.xml. I think it's because of different formating or something (I use Android Studio on Windows 10), but sorry about that!

It's an issue with editorconfig and Weblate. Android Studio has built-in support for editorconfig, and Weblate doesn't support it at all. Leave it be for now! I'll add another editorconfig file with weblate-styles for string resources later.

ashutoshgngwr · 2021-03-10T16:21:21Z

app/src/main/java/com/github/ashutoshgngwr/noice/WakeUpTimerManager.kt

+  fun getStringScheduleLeftTime(context: Context, diffHours: Int, diffMinutes: Int): String {
+    return when (diffHours) {
+      0 -> {
+        when (diffMinutes) {
+          0 -> context.getString(R.string.wake_up_timer_schedule_set_minute, diffMinutes)
+          1 -> context.getString(R.string.wake_up_timer_schedule_set_minute, diffMinutes)
+          else -> context.getString(R.string.wake_up_timer_schedule_set_minutes, diffMinutes)
+        }
+      }
+      1 -> {
+        when (diffMinutes) {
+          0 -> context.getString(R.string.wake_up_timer_schedule_set_hour, diffHours)
+          1 -> context.getString(
+            R.string.wake_up_timer_schedule_set_hour_minute,
+            diffHours,
+            diffMinutes
+          )
+          else -> context.getString(
+            R.string.wake_up_timer_schedule_set_hour_minutes,
+            diffHours,
+            diffMinutes
+          )
+        }
+      }
+      else -> {
+        when (diffMinutes) {
+          0 -> context.getString(R.string.wake_up_timer_schedule_set_hours, diffHours)
+          1 -> context.getString(
+            R.string.wake_up_timer_schedule_set_hours_minute,
+            diffHours,
+            diffMinutes
+          )
+          else -> context.getString(
+            R.string.wake_up_timer_schedule_set_hours_minutes,
+            diffHours,
+            diffMinutes
+          )
+        }
+      }
+    }
+  }


This isn't needed if you switch to Plurals. But plurals don't support multiple params so you'll have to concatenate strings together. Both ways are hell to translate. Your call.

Plural string should look something like:

<string name="...">Alarm will go off in %1$s %2$s.</string> <plurals name="hours"> <item quantity="zero"></item> <item quantity="one">%d hour</item> <item quantity="other">%d hours</item> </plurals> <plurals name="minutes"> <item quantity="zero"></item> <item quantity="one">%d minute</item> <item quantity="other">%d minutes</item> </plurals>

The code is just for demo. It may not work as is.

It's the first time I heard about plurals and this is a much better solution when it comes to reading code. So after playing with plurals I've decided to keep it and I'll update unit tests. Tell me which solution do you preffer once I update changes.

ashutoshgngwr · 2021-03-10T16:26:53Z

app/src/androidTest/java/com/github/ashutoshgngwr/noice/fragment/WakeUpTimerFragmentTest.kt

+  @Test
+  fun testSetSchedule_ShowNotifySnackBar() {
+    every { Preset.readAllFromUserPreferences(any()) } returns arrayOf(
+      mockk(relaxed = true) {
+        every { id } returns "test-preset"
+        every { name } returns "test-preset"
+      }
+    )
+    fragmentScenario.recreate()
+
+    onView(withId(R.id.set_time_button)).perform(click())
+    onView(withSubstring("Alarm will go off")).check(matches(isDisplayed()))
+  }


For this test-case to work, you need to mock WakeUpTimerManager.get() and return a timer in future.

fix(app): Change normal strings to plurals. Now code is more readable and smaller. test(app): Update unit tests.

ashutoshgngwr

Let me see if I can make plural logic cleaner. :)

ashutoshgngwr · 2021-03-11T06:34:56Z

app/src/androidTest/java/com/github/ashutoshgngwr/noice/fragment/WakeUpTimerFragmentTest.kt

+
+    every { WakeUpTimerManager.get(any()) } returns mockk {
+      every { presetID } returns "test-preset-id"
+      every { atMillis } returns System.currentTimeMillis() + 10000L
+      every { shouldUpdateMediaVolume } returns true
+      every { mediaVolume } returns 10
+    }
+


Sorry, it appears that I made the wrong suggestion. I forgot to consider that the snack bar is only shown after the "Schedule" button is clicked. You can verify this behaviour in testSetTimer() test-case. Add your assertions after the action on line 208.

https://github.com/ashutoshgngwr/noice/blob/f1492f5fc1ff8788f5cdc5b5a1d0ab555b9a8630/app/src/androidTest/java/com/github/ashutoshgngwr/noice/fragment/WakeUpTimerFragmentTest.kt#L157-L235

ashutoshgngwr · 2021-03-11T06:47:32Z

app/src/main/java/com/github/ashutoshgngwr/noice/WakeUpTimerManager.kt

+    return if (diffHours > 0 && diffMinutes > 0) {
+      val bridge = context.getString(R.string.wake_up_timer_schedule_set_bridge)
+      "$startString $stringHours $bridge $stringMinutes."
+    } else {
+      "$startString $stringHours$stringMinutes."


This concatenation will work for English, but it may not work for other languages. e.g., in my native language, the translation would be

अलार्म 1 घंटे और 20 मिनट में बजेगा।

which, if translated one word at a time, translates to

Alarm 1 hour and 20 minutes in will-ring.

I'd suggest using the original suggestion I made. Leave a placeholder in the message string to insert pluralised versions of minute and hour strings. e.g.

Alarm will go off in %1$s and %2$s.

And then you can format it with the hour and minutes plurals.

Bonus: you can also make your bridge a plural. e.g.

<plurals name="hours"> <item quantity="zero"></item> <item quantity="zero"></item> <item quantity="other">and</item> </plurals>

You're right, I didn't think about that, I'll set the string format with placeholders. About making bridge plural I've tried that but had no luck to make it work, maybe I'm doing something wrong? For now I'll set the string like you told me.

Alarm will go off in %1$s and %2$s.

I already fixed it in my local copy. Working on the test-case. Will push it soon.

ashutoshgngwr

looks good now!

ashutoshgngwr · 2021-03-11T09:58:12Z

@robercoding Thanks for putting in the work to fix these issues. I see a lot of your work was undone in refactoring. PS, discussing changes before heading into implementation and creating a PR right after your first commit will help prevent these situations. Let me know if you'd like to continue working.

robercoding added 5 commits March 10, 2021 14:45

feat(app) Now notifies user when alarm will go off with a snackbar.

e231f45

feat(app): Added multiple strings for a good syntax. Added function t…

7e6fedf

…o get the correct string to notify user the schedule left time.

test(app): Added unit tests. Fixed a small mistake when getting the c…

c2e1eb0

…orrect string.

test(app): Added UI test checking that SnackBar notifies to the user.

17019b0

clean(app): Clean up.

a74843a

ashutoshgngwr requested changes Mar 10, 2021

View reviewed changes

test(app): Add WakeUpTimerManager.get() to return timer.

f1492f5

fix(app): Change normal strings to plurals. Now code is more readable and smaller. test(app): Update unit tests.

ashutoshgngwr reviewed Mar 11, 2021

View reviewed changes

refactor(app): clean up plurals logic for time in WakeUpTimeFragment

bb3f82f

ashutoshgngwr marked this pull request as ready for review March 11, 2021 09:03

chore(git): merge branch ashutoshgngwr/master into robercoding/master

4ce7197

ashutoshgngwr approved these changes Mar 11, 2021

View reviewed changes

ashutoshgngwr merged commit 494f168 into trynoice:master Mar 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Visual confirmation when Wake Up timer is set. Request solution #502. #554

Added Visual confirmation when Wake Up timer is set. Request solution #502. #554

robercoding commented Mar 10, 2021 •

edited

Loading

codecov bot commented Mar 10, 2021 •

edited

Loading

robercoding commented Mar 10, 2021

ashutoshgngwr commented Mar 10, 2021 •

edited

Loading

ashutoshgngwr Mar 10, 2021

robercoding Mar 10, 2021

ashutoshgngwr Mar 10, 2021 •

edited

Loading

ashutoshgngwr left a comment

ashutoshgngwr Mar 11, 2021

ashutoshgngwr Mar 11, 2021

robercoding Mar 11, 2021

ashutoshgngwr Mar 11, 2021

ashutoshgngwr left a comment

ashutoshgngwr commented Mar 11, 2021

Added Visual confirmation when Wake Up timer is set. Request solution #502. #554

Added Visual confirmation when Wake Up timer is set. Request solution #502. #554

Conversation

robercoding commented Mar 10, 2021 • edited Loading

Changes

Testing

Others

codecov bot commented Mar 10, 2021 • edited Loading

Codecov Report

robercoding commented Mar 10, 2021

ashutoshgngwr commented Mar 10, 2021 • edited Loading

ashutoshgngwr Mar 10, 2021

Choose a reason for hiding this comment

robercoding Mar 10, 2021

Choose a reason for hiding this comment

ashutoshgngwr Mar 10, 2021 • edited Loading

Choose a reason for hiding this comment

ashutoshgngwr left a comment

Choose a reason for hiding this comment

ashutoshgngwr Mar 11, 2021

Choose a reason for hiding this comment

ashutoshgngwr Mar 11, 2021

Choose a reason for hiding this comment

robercoding Mar 11, 2021

Choose a reason for hiding this comment

ashutoshgngwr Mar 11, 2021

Choose a reason for hiding this comment

ashutoshgngwr left a comment

Choose a reason for hiding this comment

ashutoshgngwr commented Mar 11, 2021

robercoding commented Mar 10, 2021 •

edited

Loading

codecov bot commented Mar 10, 2021 •

edited

Loading

ashutoshgngwr commented Mar 10, 2021 •

edited

Loading

ashutoshgngwr Mar 10, 2021 •

edited

Loading