Code review comment for lp:~nik90/ubuntu-clock-app/edit-alarm-feature

> Consider making the naming on the AlarmLabel.qml consistent. The page title is
> "Label", but the Label component on top of the text field is "Alarm Label".
> Consider getting rid of the Label "Alarm Label" since it is redundant
> information. Similarly, the word "Label" is poor terminology. I know it is
> what is in the design spec, but it is a poor term for a title or name of an
> alarm.
>

I cannot change this without consulting with the with the designers about this. I would like to remove the "Alarm Label" as it is indeed redundant. Will see if I get a reply within 1-2 days. If not we should proceed to take care of this later as the current implementation does follow the design specs.

> In my opinion there should not be a default "label" for a new alarm. The user
> should simply be required to add a label. No real person makes an alarm and
> lets it be labeled "Alarm #1".
>
> The new alarm date picker should conform to the user's locale. In my locale I
> should see 7 AM, 12 PM, etc for 12 hour time.

This is a SDK bug. I have reported the issue and added a FIXME note in rev 33

>
> When selecting multiple dates for an alarm, say Monday and Tuesday, when they
> are displayed in a comma separated fashion there is no space between them.
> Instead the days are displayed as "Monday,Tuesday". It'd be best to have a
> space in between.

I am not sure how to fix this. In the output I have already added a empty string " " but it seems to be ignored. Got any ideas on how to fix this?

577 + else {
578 + return occurs + " "
579 + }

>
> There are numerous instances where "anchors" properties can be grouped.
> Consider doing so.

Fixed in rev 31