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)
Calendar
Dialogs
Tracking
(Not tracked)
RESOLVED
FIXED
1.4
People
(Reporter: Fallen, Assigned: Fallen)
Details
Attachments
(2 files, 1 obsolete file)
34.21 KB,
image/png
|
Details | |
11.69 KB,
patch
|
bv1578
:
review-
Fallen
:
ui-review+
|
Details | Diff | Splinter Review |
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.
Comment 1•13 years ago
|
||
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.
Assignee | ||
Comment 2•12 years ago
|
||
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)
Assignee | ||
Updated•12 years ago
|
Attachment #586404 -
Flags: review?(matthew.mecca) → review?(bv1578)
Assignee | ||
Comment 3•12 years ago
|
||
Updated•12 years ago
|
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: x86 → All
Comment 4•12 years ago
|
||
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-
Assignee | ||
Comment 5•12 years ago
|
||
(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.
Assignee | ||
Comment 6•12 years ago
|
||
> (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)
Assignee | ||
Updated•12 years ago
|
Attachment #604265 -
Flags: ui-review? → ui-review?(nisses.mail)
Assignee | ||
Comment 7•12 years ago
|
||
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+
Assignee | ||
Comment 8•12 years ago
|
||
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)
Comment 10•12 years ago
|
||
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-
Assignee | ||
Comment 11•12 years ago
|
||
(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+
Assignee | ||
Comment 12•12 years ago
|
||
Pushed to comm-central changeset 3ad9c88900d6
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.5
Assignee | ||
Comment 13•12 years ago
|
||
Backported to releases/comm-aurora changeset 9420ad8a9c7f
Target Milestone: 1.5 → 1.4
Updated•12 years ago
|
Attachment #604265 -
Flags: review?(matthew.mecca)
You need to log in
before you can comment on or make changes to this bug.
Description
•