Code review comment for lp://qastaging/~nik90/ubuntu-clock-app/replace-alarmsound-checkbox

Revision history for this message
Nekhelesh Ramananthan (nik90) wrote :

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.

« Back to merge proposal