Closed Bug 632355 Opened 9 years ago Closed 8 years ago

calendar-summary-dialog : attendees names overlap

Categories

(Calendar :: Dialogs, defect, trivial)

defect
Not set
trivial

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: philmsgs, Assigned: philmsgs)

Details

Attachments

(3 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.9.2.13) Gecko/20101203 Firefox/3.6.13
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.9.2.13) Gecko/20101207 Lightning/1.0b3pre Thunderbird/3.1.7

Displaying an event containing labels of participants very long is not correct from a read-only calendar.
In this case, attendees names overlap. We need to resize the dialog to see correct display.

Reproducible: Always

Steps to Reproduce:
1.create un event and add multiples attendees with long names (more than 25 car) 
2.open it from a read-only calendar (share it or check read-only on the calendar)
3.calendar-summary-dialog open and we can see that attendees names overlap.


Expected Results:  
attendees names not overlapping at first display.
Attached image names overlapping
patch v1 display attendees in one column only, with sorted names.
Comment on attachment 510577 [details] [diff] [review]
[after blockers]Patch v1 with some cosmetic improvement

Please request review from me when you upload a patch. I'll have a look at this when the beta bugs are fixed, we have to give those bugs priority at the moment so we can proceed on the next release.

On a general note, please be sure to replace tabs with spaces, if you look at the patch as uploaded, you see the big difference in spacing :)

If I see things right, the downside to this patch is that only one row of attendees is shown, regardless of how much horizontal space is available. So if you have resized the dialog, we can't take advantage of it.
Attachment #510577 - Flags: review?(philipp)
Assignee: nobody → philippe.martinak
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #510577 - Attachment description: Patch v1 with some cosmetic improvement → [after blockers]Patch v1 with some cosmetic improvement
Attached patch Fix - v2Splinter Review
Philippe, what do you think of this alternative? It keeps two columns and just crops the label.

This patch also hacks a fix for a XUL issue that the listbox columns don't contract once they have been made wider.
Attachment #510577 - Attachment is obsolete: true
Attachment #590694 - Flags: review?(philippe.martinak)
Attachment #510577 - Flags: review?(philipp)
(In reply to Philipp Kewisch [:Fallen] from comment #6)
> Created attachment 590694 [details] [diff] [review]
> Fix - v2
> 
> Philippe, what do you think of this alternative? It keeps two columns and
> just crops the label.
> 
> This patch also hacks a fix for a XUL issue that the listbox columns don't
> contract once they have been made wider.

I think the two solutions are good. 
we use one column because some user have a long display name (more than 50 chars).

Fix - v2:

if (attendee.commonName && attendee.commonName.length) {
  label.value = attendee.commonName;
  // XXX While this is correct from a XUL standpoint, it doesn't
  // seem to work on the listcell. Working around this would be an
  // evil hack, so I'm waiting for it to be fixed in the core
  // code instead.
  listcell.setAttribute("tooltiptext", attendee.toString());
  
=>  label is not defined (removed by the patch)

listcell.setAttribute("tooltiptext", attendee.toString());
=>
listcell.setAttribute("label",  attendee.commonName);
?
(In reply to Philipp Kewisch [:Fallen] from comment #6)
> Created attachment 590694 [details] [diff] [review]
> Fix - v2
> 
> Philippe, what do you think of this alternative? It keeps two columns and
> just crops the label.
> 
> This patch also hacks a fix for a XUL issue that the listbox columns don't
> contract once they have been made wider.

with the v2 fix, if the event has three participants, a fourth icon is visible.

<listcell class="listcell-iconic status-icon" crop="end"/>
<listcell class="listcell-iconic status-icon" crop="end"/>
=>
<listcell class="listcell-iconic status-icon" crop="end" hidden="true"/>
<listcell class="listcell-iconic status-icon" crop="end" hidden="true"/>


listcell.setAttribute("status", attendee.participationStatus);
page++;
=>
listcell.setAttribute("status", attendee.participationStatus);
listcell.removeAttribute("hidden");
page++;
Component: General → Dialogs
OS: Windows XP → All
QA Contact: general → dialogs
Hardware: x86 → All
Comment on attachment 590694 [details] [diff] [review]
Fix - v2

I've fixed the issues you mentioned, thank you very much! I understand your reasoning with the one column, I think in the long run it should be fixed differently though:

It would be nice if the dialog could layout the attendees automatically. It would be cool if we could just add the attendees to a hbox and use something (maybe -moz-column-count and friends) to automatically sort them into as many columns as fit,  but some more logic might be needed to make sure it doesn't look strange if only one or two attendees have a very long name.

I recall working on a bug to fix that for the attendee tooltip some time back in 2007, but I resigned because at that point it was too complicated.
Attachment #590694 - Flags: review?(philippe.martinak) → review+
Pushed to comm-central changeset b0f2bbc5d752
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4
You need to log in before you can comment on or make changes to this bug.