Closed Bug 689130 Opened 13 years ago Closed 12 years ago

Enabling timezone support in the event dialog should be more visible

Categories

(Calendar :: Dialogs, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Fallen, Assigned: Fallen)

Details

Attachments

(2 files, 1 obsolete file)

Right now the timezone option is disabled by default, so we have a wide gap of empty space on the right side of the date picker.

Also, its not intuitive for users requiring timezones on how to enable them, it may seem like we don't support different timezones per event at all.

So to fix this issue, there are at least two options:

1) Add some UI in the event dialog that makes it more obvious on how to enable timezones

2) Flip the default so timezones can be disabled if not needed, but are enabled by default.
Not sure about #2. Conversely, I would have thought that most of our users do not need TZ, but a collapsible pane for TZ would allow people who need it to have access to it quickly. Therefore, I'd think the TZ pane should be collapsed by default.

Ideally, the status of the TZ pane should be remembered from one event display to the other.
Attached patch Fix - v1 (obsolete) β€” β€” Splinter Review
After some discussions with jb about the original intent of this bug, I've created this patch. Instead of flipping timezone display bits, it just enhances timezone usage when its enabled.

Now, when you click on the timezone name, it shows a popup with the default timezone and 5 other recently chosen timezones. It also allows you to open the conventional dialog.

If we do want to make enabling timezones more obvious, we might be able to use the empty space in the time picker. The problem here is that the time picker itself is a finished widget, so if we want to use the empty area there we'd have to hack it in which could get ugly.

The state of timezones being enabled should be persisted all along.
Assignee: nobody → philipp
Attachment #586404 - Flags: ui-review?(nisses.mail)
Attachment #586404 - Flags: review?(matthew.mecca)
Attachment #586404 - Flags: review?(matthew.mecca) → review?(bv1578)
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: x86 → All
Comment on attachment 586404 [details] [diff] [review]
Fix - v1

Some small remarks:
* The checkbox in the View menu says "Timezones", I think it would be better to call it something along the way of "Show timezones" to make it more of an action.
* I don't think "Custom" is the right word in the dropdown. Not quite sure what would be a better fit, but I would suggest "Other"
* The dropdown styling is different from what we use elsewhere. I think it should be a button on hover similar to what we do with "Other actions" in the TB message header.
* The dialog was slightly too small to fit "Europe/Stockholm" on my Linux system.
Attachment #586404 - Flags: ui-review?(nisses.mail) → ui-review-
(In reply to Andreas Nilsson (:andreasn) from comment #4)
> Comment on attachment 586404 [details] [diff] [review]
> Fix - v1
> 
> Some small remarks:
> * The checkbox in the View menu says "Timezones", I think it would be better
> to call it something along the way of "Show timezones" to make it more of an
> action.
Good idea, lets change this on trunk.

> * I don't think "Custom" is the right word in the dropdown. Not quite sure
> what would be a better fit, but I would suggest "Other"
I did this mostly to avoid string changes so we can backport to comm-aurora. I think "More..." would be best for comm-central.

> * The dropdown styling is different from what we use elsewhere. I think it
> should be a button on hover similar to what we do with "Other actions" in
> the TB message header.
The dropdown styling matches what we have for attendees. I fear that if we style this as a button, the horizontal space would be even less since we'd need an empty margin when its not a button. What do you think?

> * The dialog was slightly too small to fit "Europe/Stockholm" on my Linux
> system.
I bet this has been that way before too. Maybe its time to put the timezone name into a new line? This could cause confusion though as to which timezone matches which date/time. The other option would be to increase the default width of the dialog, although this won't help users that have already modified the width of the dialog, unless we change the ID of the dialog and reset the custom resizing.
Attached patch Fix - v2 β€” β€” Splinter Review
> (In reply to Andreas Nilsson (:andreasn) from comment #4)
> > * The checkbox in the View menu says "Timezones", I think it would be better
> > to call it something along the way of "Show timezones" to make it more of an
> > action.
> Good idea, lets change this on trunk.
Done


> > * I don't think "Custom" is the right word in the dropdown. Not quite sure
> > what would be a better fit, but I would suggest "Other"
> I did this mostly to avoid string changes so we can backport to comm-aurora.
> I think "More..." would be best for comm-central.
Changed to "More Timezones", will keep at "Custom" for aurora.


> > * The dropdown styling is different from what we use elsewhere. I think it
> > should be a button on hover similar to what we do with "Other actions" in
> > the TB message header.
> The dropdown styling matches what we have for attendees. I fear that if we
> style this as a button, the horizontal space would be even less since we'd
> need an empty margin when its not a button. What do you think?
Lets keep this as is for now, if you think the attendee area should be redesigned together with this link, lets file a followup bug.

> > * The dialog was slightly too small to fit "Europe/Stockholm" on my Linux
> > system.
Also rather part of a followup bug. The dialog has no default width, so the initial content width is used. I could imagine splitting things up so that in the dialog the timezone is just "Stockholm" with a tooltip "Europe/Stockholm" 



I'd love to have this bug in on aurora before the merge on Tuesday, so I'd appreciate a speedy review.
Attachment #586404 - Attachment is obsolete: true
Attachment #586404 - Flags: review?(bv1578)
Attachment #604265 - Flags: ui-review?
Attachment #604265 - Flags: review?(bv1578)
Attachment #604265 - Flags: ui-review? → ui-review?(nisses.mail)
Comment on attachment 604265 [details] [diff] [review]
Fix - v2

Andreas told me via IRC its fine to solve the remaining issues in follow-up bugs, therefore ui-r+.
Attachment #604265 - Flags: ui-review?(nisses.mail) → ui-review+
Comment on attachment 604265 [details] [diff] [review]
Fix - v2

Hey Matthew, in case Decathlon doesn't have time, do you think you could review this by tomorrow? I'd like to push it before the merge so we can get it in a release earlier.
Attachment #604265 - Flags: review?(matthew.mecca)
I'm reviewing the patch in this moment.
Comment on attachment 604265 [details] [diff] [review]
Fix - v2

The patch works fine but there is an issue (and a possible one).

1) when the event is "all day", the first timezone link allows to open the popup menu and change the timezone instead it should be deactivated. To get this issue, first you have to open the timezone popup menu then you have to check the "all day" checkbox.
The problem occurs only to the first of the two timezone links.

2) Since the list of timezones is accessible to the user via a preference, I think this code

>+    let recentTimezones = JSON.parse(cal.getPrefSafe("calendar.timezone.recent", "[]"));

should be a bit more robust at least to avoid the case of a complete deleting of the string via the config. editor. I've tried to delete the string in the preference with the idea of a reset of the timezones list and the popup menu has not worked anymore. The console reported the error:

JSON.parse: unexpected end of data

and in this case for a user would be difficult to restore the normal functioning.
I know, it's a particular case, but it needs a few code lines to avoid it.


r- only for these issues, apart from that it seems working fine in every situation.
Attachment #604265 - Flags: review?(bv1578) → review-
(In reply to Decathlon from comment #10)
> 1) when the event is "all day", the first timezone link allows to open the
> popup menu and change the timezone instead it should be deactivated. To get
> this issue, first you have to open the timezone popup menu then you have to
> check the "all day" checkbox.
> The problem occurs only to the first of the two timezone links.
Fixed. Strange that it didn't also happen for the second one. Anyway, I added a check for the disabled state in the popup handler.


> deleting of the string via the config. editor. I've tried to delete the
> string in the preference with the idea of a reset of the timezones list and
The "Reset" menuitem will reset the preference to something useful, but I've added 7 characters that will also make an empty string work :-)


> r- only for these issues, apart from that it seems working fine in every
> situation.
I'm going to push the patch now due to the merge coming up, but I'll upload it again so you can r+
Pushed to comm-central changeset 3ad9c88900d6
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.5
Backported to releases/comm-aurora changeset 9420ad8a9c7f
Target Milestone: 1.5 → 1.4
Attachment #604265 - Flags: review?(matthew.mecca)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: