Closed Bug 984105 Opened 6 years ago Closed 6 years ago

Undisclosed Attendee checkbox should be hidden instead of disabled.

Categories

(Calendar :: E-mail based Scheduling (iTIP/iMIP), defect)

Lightning 2.6
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Fallen, Assigned: MakeMyDay)

Details

Attachments

(2 files, 2 obsolete files)

bug 544169 introduced the undisclosed invitations feature, but (rightfully) disables it in some cases. Instead of disabling the feature, I think it should rather be hidden. This way users won't complain that they can't check the checkbox, its hard to understand why its not enabled.

Do you agree, and if so, can you create the patch?
Oh, I get it, it was disabled when the notify attendees checkbox is unchecked. Does this need to be exclusive? Of course, invitations won't be sent if notify attendees is off, but does it really need to disable/enable the undisclosed checkbox?
Of course it's a matter of taste, whether the checkbox is disabled or hidden. If we hide it in cases it is not applicable, we would need to unhide it if the circumstances change (as in comment #1), which would shift all UI elements below downwards. Wouldn't this make e.g. the status bar of the dialog unvisible?

There should be a tooltip visible explaining the feature. Maybe its text can be made more verbose?
Richard, what are your thoughts on the UI here?
Flags: needinfo?(richard.marti)
I think it makes sense to disable the undisclosed invitations when notify attendees is unchecked. Like MakeMyDay said hiding/unhiding would let the content jump.

I propose the undisclosed invitations should be indented to the notify attendees then it makes it clearer it's not a separate option but dependent on notify attendees.
Flags: needinfo?(richard.marti)
The patch puts the undisclosure checkbox to the same row and to the right of the notifify attendees checkbox. It keeps the disabling behaviour, even though hidding wouldn't move content anymore.
Attachment #8391956 - Flags: review?(philipp)
Comment on attachment 8391956 [details] [diff] [review]
UnifyNotificationCheckboxesInOneRowInEventDialog-V1.diff

I'd like to see how this looks, but my build is broken. I'll do a rebuild tomorrow. If I haven't commented and you have a sec, could you upload a screenshot? I fear this might take too much horizontal space if its next to each other.
Attached image undisclosed-checkbox.png β€”
maybe the might get critical - not in the English version, but may be in localized versions. The screenshot shows the dialog with minimum width. Maybe we can make the string in the English version more concise (like: notify undisclosed) to make it obvious to localizers to do so in their language version and/or add a regarding comment to the dtd file?

Otherwise, I tend to keep everything as is - a more indented checkbox in the second row would look odd as well.
I thing for a shorter string "Separate invitation per attendee" might work. A tooltip could then be used to provide a more verbose explanation.
Thanks for the screenshot, I do like the side by side look. MakeMyDay, if you agree, maybe you can put up a patch that uses Reid's suggestion and a tooltip that explains it in more detail? Suggested tooltip:


    This option sends one invitation email per attendee. Each invitation only contains the recipient
    attendee so that other attendee identities are not disclosed.


To make sure localizers know what to do in case their text is too long, maybe you can add a localization note too.

Sorry if I am being pedantic here, but when I found this feature I initially didn't know how to enable it. I knew what it does because I know how invitations work, but I'm not sure this is clear to everyone.
Ok, I will take care. There should be already appear a tooltip (with different text) in the current version, doesn't it show up to you?
Updated patch with suggested strings.
Attachment #8391956 - Attachment is obsolete: true
Attachment #8391956 - Flags: review?(philipp)
Attachment #8393168 - Flags: review?(philipp)
Comment on attachment 8393168 [details] [diff] [review]
UnifyNotificationCheckboxesInOneRowInEventDialog-V2.diff

Review of attachment 8393168 [details] [diff] [review]:
-----------------------------------------------------------------

r=philipp

::: calendar/locales/en-US/chrome/calendar/calendar-event-dialog.dtd
@@ +12,2 @@
>  
> +<!-- note to localizers: notify.label and notifiundisclosed.label

There is a predefined format for localization notes, I will change this before checkin.

@@ -13,3 @@
>  
> -<!ENTITY newevent.attendees.notifyundisclosed.label    "Keep attendee undisclosed in invitations">
> -<!ENTITY newevent.attendees.notifyundisclosed.tooltip  "If enabled, attendees will not see each other in invitations. Applied only on e-mail invitations.">

Sorry, it looks like I didn't actually check the tooltip, my fault. I still think the new text describes the situation a little better.
Attachment #8393168 - Flags: review?(philipp) → review+
Attached patch Fix - v3 β€” β€” Splinter Review
Patch for checkin with full headers, tree is still closed
Attachment #8393168 - Attachment is obsolete: true
Attachment #8394492 - Flags: review+
https://hg.mozilla.org/comm-central/rev/4364bdeed601
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → 3.3
One thing I forgot: when the wording or meaning of an entity is changed, the name of the entity needs to be changed too. Sorry for not mentioning this.

I've fixed it in https://hg.mozilla.org/comm-central/rev/5c4f8cf4c147
You need to log in before you can comment on or make changes to this bug.