GNOME Bugzilla – Bug 681949
Add subtexts
Last modified: 2012-08-17 20:18:10 UTC
Created attachment 221320 [details] [review] Patch which adds both. Add Yesterday/Tomorrow to clocks if necessary, and the days of repeats of alarms.
Created attachment 221369 [details] [review] Updated for file changes File changes to make clocks installable made it not apply cleanly, so updated.
(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
Created attachment 221621 [details] [review] Updated again Done
Hi Alex, thanks for updating the patch. Can you explain a bit in more what it does? Maybe also provide a screenshot
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.
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!
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
(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!
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.
Created attachment 221643 [details] New screenshot I think the "Repeats" is just less useful.
(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.
Alex can you deprecate all the patches and squash them into one patch :D awesome work btw
Created attachment 221648 [details] [review] With medium text size and "fancy" repeats Squashed into one and obsoleted the rest
Created attachment 221664 [details] [review] Fixed long lines
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!
Yes, thanks! This is great.