After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 681949 - Add subtexts
Add subtexts
Status: RESOLVED FIXED
Product: gnome-clocks
Classification: Applications
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Clocks maintainer(s)
Clocks maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2012-08-15 21:31 UTC by Alex Anthony
Modified: 2012-08-17 20:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch which adds both. (10.60 KB, patch)
2012-08-15 21:31 UTC, Alex Anthony
none Details | Review
Updated for file changes (10.31 KB, patch)
2012-08-16 13:30 UTC, Alex Anthony
none Details | Review
Updated again (10.20 KB, patch)
2012-08-17 15:15 UTC, Alex Anthony
none Details | Review
Alarms window (42.59 KB, image/png)
2012-08-17 16:00 UTC, Alex Anthony
  Details
Added Allan's design changes (10.12 KB, patch)
2012-08-17 17:11 UTC, Alex Anthony
none Details | Review
New screenshot (39.90 KB, image/png)
2012-08-17 17:13 UTC, Alex Anthony
  Details
With medium text size and "fancy" repeats (10.19 KB, patch)
2012-08-17 17:45 UTC, Alex Anthony
none Details | Review
Fixed long lines (11.15 KB, patch)
2012-08-17 18:24 UTC, Alex Anthony
none Details | Review

Description Alex Anthony 2012-08-15 21:31:33 UTC
Created attachment 221320 [details] [review]
Patch which adds both.

Add Yesterday/Tomorrow to clocks if necessary, and the days of repeats of alarms.
Comment 1 Alex Anthony 2012-08-16 13:30:01 UTC
Created attachment 221369 [details] [review]
Updated for file changes

File changes to make clocks installable made it not apply cleanly, so updated.
Comment 2 Seif Lotfy 2012-08-17 10:25:45 UTC
(In reply to comment #1)
> Created an attachment (id=221369) [details] [review]
> Updated for file changes
> 
> File changes to make clocks installable made it not apply cleanly, so updated.

Thanks for the patch. Sadly I can't test it since it doesnt apply on the latest master. Do you think you can rebase maybe, would be really awesome.
Thanks
Seif
Comment 3 Alex Anthony 2012-08-17 15:15:53 UTC
Created attachment 221621 [details] [review]
Updated again

Done
Comment 4 Paolo Borelli 2012-08-17 15:37:48 UTC
Hi Alex, thanks for updating the patch. Can you explain a bit in more what it does? Maybe also provide a screenshot
Comment 5 Alex Anthony 2012-08-17 16:00:47 UTC
Created attachment 221624 [details]
Alarms window

In https://live.gnome.org/Design/Apps/Clock, the mockups show Tomorrow on a world clock if they're ahead enough to have reached that point. Also, repeating alarms show when they're repeating there.

I've attached a screenshot of the alarms. It also does the Tomorrow/Yesterday bit, and extends the background overlay rectangle to contain it.

Also of note, the logic for the day/night on alarms is wrong. It uses am=day, pm=night.
Comment 6 Allan Day 2012-08-17 16:35:01 UTC
Hey Alex! Thanks for working on this - it's most needed, and this looks really good. I think we should be able to land it with a few tweaks.

A few things:

 * I think that "Everyday" is fantastic! I'm less sure about "Weekends" though (it isn't clear whether that alarm goes off once every weekend or not), and the specific "Mon, Wed, Fri" is likely to scale badly. So, I wonder if we should restrict ourselves to a more limited set of options here:
  - If it repeats on one day of the week, print the the day
  - "Everyday" if it repeats 7 days a week
  - "Weekdays" if it repeats Monday - Friday
  - If any other combination of days, just display "Repeats"

 * Would it be possible to use the full names of days, rather than their abbreviations? (eg. "Friday" instead of "Fri")

 * Could you try putting the subtexts in a regular font weight, rather than bold?

Thanks again!
Comment 7 Alex Anthony 2012-08-17 16:58:05 UTC
Removing weekends is easy, will do.

"Everyday" or "Every day"? (My android phone uses the latter)

I added logic for allowing the text to wrap, so it can handle 6 days of repeat ok (7 being unnecessary as its every day) - see the last clock. This will look better once it's regular weight too. If one alarm says "Repeats", and another says "Weekdays", it sort of suggests that the Weekdays one isn't repeating.
And it's better to be able to view that at a glance rather than having to go into an edit dialog.

And obviously the abbreviations were used so 6 days could fit

I'll make up a patch as you've suggested
Comment 8 Allan Day 2012-08-17 17:10:04 UTC
(In reply to comment #7)
> Removing weekends is easy, will do.
> 
> "Everyday" or "Every day"? (My android phone uses the latter)

Yes, "Every day" is better!

> I added logic for allowing the text to wrap, so it can handle 6 days of repeat
> ok (7 being unnecessary as its every day) - see the last clock. This will look
> better once it's regular weight too. If one alarm says "Repeats", and another
> says "Weekdays", it sort of suggests that the Weekdays one isn't repeating.
> And it's better to be able to view that at a glance rather than having to go
> into an edit dialog.
> 
> And obviously the abbreviations were used so 6 days could fit

It seems like a lot of information to list each day, is all, plus I'd prefer to use full names for the days.

> I'll make up a patch as you've suggested

Thanks!
Comment 9 Alex Anthony 2012-08-17 17:11:44 UTC
Created attachment 221641 [details] [review]
Added Allan's design changes

This is it updated for the design changes. I'll add a new screenshot showing the changes.

Changed to Every Day

And we use the shortened names in the add alarm dialog anyway.
Comment 10 Alex Anthony 2012-08-17 17:13:10 UTC
Created attachment 221643 [details]
New screenshot

I think the "Repeats" is just less useful.
Comment 11 Allan Day 2012-08-17 17:23:14 UTC
(In reply to comment #10)
> Created an attachment (id=221643) [details]
> New screenshot
> 
> I think the "Repeats" is just less useful.

OK, let's go with the more elaborate labels for now then. :) Maybe try making the subtext slightly smaller?

Anyway, this looks good. I'll hand over to someone else for code review.
Comment 12 Seif Lotfy 2012-08-17 17:26:44 UTC
Alex can you deprecate all the patches and squash them into one patch :D
awesome work btw
Comment 13 Alex Anthony 2012-08-17 17:45:01 UTC
Created attachment 221648 [details] [review]
With medium text size and "fancy" repeats

Squashed into one and obsoleted the rest
Comment 14 Alex Anthony 2012-08-17 18:24:04 UTC
Created attachment 221664 [details] [review]
Fixed long lines
Comment 15 Paolo Borelli 2012-08-17 19:08:19 UTC
Committed with a couple of minor code tweaks (in particular we need to import Pango, that was recently remove because not used).

If needed we can always refine the look of the sub text incrementally.

Thanks a lot for your work!
Comment 16 Allan Day 2012-08-17 20:18:10 UTC
Yes, thanks! This is great.