984105 - Undisclosed Attendee checkbox should be hidden instead of disabled.

Status: RESOLVED FIXED

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

Description

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

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?

Comment 1

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

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?

Comment 2

[[:MakeMyDay]]

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?

Comment 3

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Richard, what are your thoughts on the UI here?

Comment 4

[Richard Marti (:Paenglab)]

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.

Comment 5

[[:MakeMyDay]]

Attachment: UnifyNotificationCheckboxesInOneRowInEventDialog-V1.diff

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.

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

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.

Comment 7

[[:MakeMyDay]]

Attachment: 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.

Comment 8

[Reid Anderson]

I thing for a shorter string "Separate invitation per attendee" might work. A tooltip could then be used to provide a more verbose explanation.

Comment 9

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

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.

Comment 10

[[:MakeMyDay]]

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?

Comment 11

[[:MakeMyDay]]

Attachment: UnifyNotificationCheckboxesInOneRowInEventDialog-V2.diff

Updated patch with suggested strings.

Comment 12

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

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.

Comment 13

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Fix - v3

Patch for checkin with full headers, tree is still closed

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/4364bdeed601

Comment 15

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

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