Merge lp://qastaging/~nik90/ubuntu-clock-app/replace-alarmsound-checkbox into lp://qastaging/ubuntu-clock-app

PASSED: Continuous integration, rev:369
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/817/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/162

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/817/rebuild

Comments:

1. Please update the pot file for the new strings.
2. Could we indent the alarm sound delegate text? This might help to make the section headings more obvious? This is just an opinion, however.
3. As part of this MP could you make it so the when the user selects the original alarm sound in the AlarmSound page that the saveAction is still enabled? It's weird if you play a few and get back to the original and you can't save it. IMO it should always be enabled.
4. I wonder if it'd be useful to put a small play/pause toggle next to the tick box. This would make it more apparent that tapping the item again will stop the alarm playback.

Only the first issue needs to be fixed.

Additional comment:

Consider changing "Default sounds" and "Custom sounds" to "Default alarms" and "Custom alarms". Or maybe "Custom alarm sounds", etc? My prefered Android alarm clock uses "alarm sound" as the term.

If you do change the headings per that last comment--you may also want to change "Add custom sound" and "Add sound from" to match.

Also, maybe "Custom Alarm Sounds" should be "Custom alarm sounds" (note capitalization).

> Comments:
>
> 1. Please update the pot file for the new strings.

Will do once we agree on the wording of the headers to be used.

> 2. Could we indent the alarm sound delegate text? This might help to make the
> section headings more obvious? This is just an opinion, however.

I agree that the section headings need to be more obvious and I am a bit conflicted on how to proceed. If I indent the alarm sound delegate text, it breaks consistency with section headers shown by the address-book-app and the world city page in the clock app. Right now, the page looks very busy with so many listitem dividers. I will try out a couple different variations and see which one maintains consistency, clean loook etc.

> 3. As part of this MP could you make it so the when the user selects the
> original alarm sound in the AlarmSound page that the saveAction is still
> enabled? It's weird if you play a few and get back to the original and you
> can't save it. IMO it should always be enabled.

Hmm I can see how it gets annoying after you makes changes and then revert back to the original sound. At the moment, the save button is enabled *only* when you change the value to something new. We follow this pattern everywhere now. I suppose I can change that to enabling the save button if *any* change is done regardless of whether that changes defaults to the old/new value. I will have to do this in 3 different places and also fix the unit tests. A separate MP is a better idea.

> 4. I wonder if it'd be useful to put a small play/pause toggle next to the
> tick box. This would make it more apparent that tapping the item again will
> stop the alarm playback.
>

Hmm good idea. I will add this.

As for the naming, the page title where we show "add sound from" can accommodate only up to 3 strings beyond which it ellipses due to lack of space (especially when translated).

I like to use "Default alarm sounds" and "Custom alarm sounds".

So the button would also need to be changed to "Add custom alarm sound". While the page title stays "add sound from" due to space constraints.

Does this sound good?

- In my opinion "Add custom sound" button should be on inside "Custom Sounds" list, as it is belongs it it.
- In my opinion "Custom Sounds" and "Default Sounds" should be always visible
- After that "Add custom sound" could be renamed to just "Add sound" as this button is already in "Custom Sounds"

Use cases:
- When there is no custom sounds, the header "Custom Sounds" will be displayed, and "Add sound" will be there
- When there is custom sounds, the header "Custom Sounds" will be displayed, and "Add sound" will be on top of the list. Next it will be custom alarms sounds.

If you want I could create some mockup for you.

One more comment:
- After adding new Custom Sound it will be great to show simple add animation, as it is already done for "Stopwatch -> Lap". It will visualize what was changed in the list.

@Victor Where we should use capital letters an where not: I mean "Custom Sounds" vs "Custom sounds" ?

Some small coding style remarks.

Okay, I fixed almost all of the feedback/issues pointed out except for the following,

> 1. - After adding new Custom Sound it will be great to show simple add animation, as it is already done for "Stopwatch -> Lap". It will visualize what was changed in the list.

This is not easy to do since we use Repeater+Column which doesn't provide us with onDisplaced signal to animate the addition/deletion of sounds.

> 2. Could we indent the alarm sound delegate text? This might help to make the
> section headings more obvious? This is just an opinion, however.

Indentation will break the consistency. So I would rather keep it this way by making the section headers text semibold.

> 3. As part of this MP could you make it so the when the user selects the
> original alarm sound in the AlarmSound page that the saveAction is still
> enabled? It's weird if you play a few and get back to the original and you
> can't save it. IMO it should always be enabled.

As mentioned earlier, this deserves its own MP since it affects 3 other files. Let's do it in one go.

PASSED: Continuous integration, rev:372
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/818/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/163

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/818/rebuild

NOTE: Please do not merge this MP before we push out the clock-app 3.5 store update since this MP will update the pot file and change strings that I won't be able to bisect from the release. QA tested and approved rev 361. I will merge upcoming spanish and chinese translation which requires the .pot file not be updated.

FAILED: Continuous integration, rev:373
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/819/
Executed test runs:
    FAILURE: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/164/console

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/819/rebuild

PASSED: Continuous integration, rev:374
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/821/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/166

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/821/rebuild

It is working perfectly for me.

Everything is fine according to checklist.
Thanks.

MP ready to be merged now that v3.5 has been released.

PASSED: Continuous integration, rev:377
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/826/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/171

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/826/rebuild