258986 - Cleanup event & todo/task mouseover preview tooltips: combine code, add properties, date ranges

Status: RESOLVED FIXED

(Calendar :: Sunbird Only, defect)

Description

[gekacheka]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; rv:1.7.3) Gecko/20040910 Firefox/0.10
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; rv:1.7.3) Gecko/20040910 Firefox/0.10, 20040910-cal

Mouseover tooltip code is spread out and duplicated in calendar.js and
unifinder.js.  Some behavior is inconsistent, e.g., event locations are shown
but not todo location; allday date formats are different depending on mouse over
event list or mouse over event in grid.  Displaying multiline descriptions looks
ugly.

Reproducible: Always
Steps to Reproduce:
1. mouseover events in test file


Actual Results:  
Multiline description: first line has "Note: " prepended, others do not.
Task with location: no location displayed.
Other properties not displayed.
Start and end date are displayed on separate lines, could be combined into a
range as in export to html or email events.

Expected Results:  
Multiline description: display like body of message, below headers after blank line.
Task with location: location displayed.
Other properties displayed.
Start and end date combined into a range as in export to html or email event.

Comment 1

[gekacheka]

Attachment: test cases: Calendar with test events and todo/tasks to mouseover

Comment 2

[gekacheka]

Attachment: content patch: combines code from calendar.js and unifinder.js to mouseoverPrevious.js

mouseoverPreviews.js (new file)
  From calendar.js:
    gShowTooltip   [was showTooltip]
    checkTooltip()
    getPreviewForTask(todo)  
      [was getPreviewTextForTask]
      Code simplified with helper functions; 
      Location, etc. properties added
    getPreviewForEventDisplay(eventDisplay) 
      [was getPreviewTextForRepeatingEvent]
      duplicated code eliminated by calling getPreviewForEvent.
  From unifinder.js
    getPreviewForEvent(...)
      [was getPreviewText]
      Code simplified with helper functions.
      Date ranges added for start -- end date.
      Status property added.
    getNextOrPreviousRecurrence [cleaner version from calendarEvent.js]
  New helper functions:
    getEventStatusString
    getToDoStatusString
    boxAppendText
    boxAppendLines
    boxAppendLabeledText
    boxAppendLabeledDateTime
    boxAppendLabeledDateTimeInterval

calendar.xul, calendar.xul (sunbird)
  Add script mouseoverPreviews.js

calendar.js
  Moved to mouseoverPreviews.js:
    showTooltip 		    [to gShowTooltip]
    checkTooltip
    getPreviewTextForTask	    [to getPreviewForTask]
    getPreviewTextForRepeatingEvent [to getPreviewForEventDisplay]
  Removed (unused):
    getNumberOfRepeatTimes

calendarWindow.js
  changeMouseOverInfo:
    renamed getPreviewTextForTask to getPreviewForTask
    renamed getPreviewTextForRepeatingEvent to getPreviewForEvent
    renamed Html to tipNode

unifinder.js
  Modified:
    treeView.getCellText: "...tree-col-status"
      Code simplified with call to getEventStatusString.
    changeTooltipTextForEvent
      Code simplified with call to getPreviewForEvent.
      renamed Html to toolTip
  Moved to mouseoverPreviews.js
    getNextOrPreviousRecurrence
    getPreviewText		    [to getPreviewForEvent]

unifinderToDo.js
  Modified:
    changeToolTipTextForToDo:
      renamed getPreviewTextForTask to getPreviewForTask
      renamed Html to toolTip

Comment 3

[gekacheka]

Attachment: calendar.properties patch, all locales: add todo statuses, todo tooltip properties

calendar.properties, all locales:
  Statuses
    Rename status properties with "status" prefix
    Add properties for todo status names
  Tooltips
    Added properties for priority, percent, completed date
    Added property tooltipDate for Date: (for event date interval)
    Removed unused tooltip properties.
    Translations copied from calendar.dtd

Comment 4

[gekacheka]

Attachment: test cases: Calendar with test events and todo/tasks to mouseover (add repeating event)

Comment 5

[gekacheka]

Attachment: content patch: combines code from calendar.js and unifinder.js to mouseoverPreviews.js (updated)

As in comment #2.
patch updated for/depends on:
    bug258956
      renamed getNextOrPreviousRecurrence to getCurrentNextOrPreviousRecurrence

    bug260169
      modified formatInterval to add relativeToDate
      modified formatDateTime to receive true for allDayOrTimeString

Comment 6

[Mostafa Hosseini]

Comment on attachment 158663 [details] [diff] [review]
calendar.properties patch, all locales:  add todo statuses, todo tooltip properties

Correct me if I'm wrong but from my understanding .properties files can't
contain special characters so they have to be encoded in unicode. Copying them
from the .dtd file ( as in the case of statusNeedsAction for example ) is not
right

Comment 7

[gekacheka]

While it is true that Java .properties files specify ISO8859-1, I think UTF-8 is
ok for mozilla .properties.  The first thing it does is convert from UTF-8:
http://lxr.mozilla.org/mozilla/source/xpcom/ds/nsPersistentProperties.cpp#146

For future reference, this and other differences are listed in the user note at
http://www.xulplanet.com/tutorials/xultu/locprops.html

Comment 8

[Mostafa Hosseini]

(In reply to comment #7)
> While it is true that Java .properties files specify ISO8859-1, I think UTF-8 is
> ok for mozilla .properties.  The first thing it does is convert from UTF-8:
> http://lxr.mozilla.org/mozilla/source/xpcom/ds/nsPersistentProperties.cpp#146
Thanks for the clarification. Regarding the same attachment, I'm assuming that
the fact that in some locales( e.g. pt-BR ) the old tooltip settings have not
been removed ( e.g. tooltipTitleElement ) is by mistake. Let me know otherwise.

Comment 9

[gekacheka]

You are right, there are bugs.  I'll provide updated patches later.

Comment 10

[Mostafa Hosseini]

(In reply to comment #9)
> You are right, there are bugs.  I'll provide updated patches later.

That's alright. I'm taking care of them while doing the review.

Comment 11

[gekacheka]

Attachment: content patch: combines code from calendar.js and unifinder.js to mouseoverPreviews.js (updated)

Updated.
  
Adds bold, aligned headings for fields.

Uses oeICalDate_isSet to test whether dates are set.
(ieICalDate_isSet is temporary until oeICalDate.isSet is implemented as
described in bug 220075 comment 21 )

Comment 12

[gekacheka]

Attachment: calendar.properties patch, all locales: add todo statuses, todo tooltip properties (updated)

Updated patch.	Headers are now displayed in a table so they can be aligned, so
no longer uses string parameters.

Comment 13

[gekacheka]

Tested on Sunbird 20041008, Moz1.7.3, TB 0.8, FF 0.10.1

Mostafah, since you didn't finish,
Please use these new patches instead.  The code adds the features mentioned
(using bold headings aligned in a table, using oeICalDate_isSet to test whether
value is set), and the locale properties no longer use parameters, but otherwise
it is the same.

Comment 14

[Mostafa Hosseini]

Comment on attachment 163358 [details] [diff] [review]
calendar.properties patch, all locales: add todo statuses, todo tooltip properties (updated)

The diff for en-US and it-IT looks wrong

Comment 15

[Mostafa Hosseini]

Comment on attachment 163357 [details] [diff] [review]
content patch: combines code from calendar.js and unifinder.js to mouseoverPreviews.js (updated)

How come getFormattedString is not used anymore? It looks like it is missing
from the new functions that have replaced the old commented out
boxAppendLabeledText function.

Comment 16

[gekacheka]

Attachment: calendar.properties patch, all locales: add todo statuses, todo tooltip properties (updated)

I'm sorry, it looks like I didn't update all locales to remove the parameters.
The en-US is correct, this patch updates the rest of the locales as well.

The update makes the tooltips easier to read by aligning the header names and
values in a grid, and making the name bold.  Since the names and values are in
different cells of the grid, getFormattedString is no longer used.  (They were
all the same,  
 Name:	Value 
.)

Comment 17

[Mostafa Hosseini]

Comment on attachment 163437 [details] [diff] [review]
calendar.properties patch, all locales: add todo statuses, todo tooltip properties (updated)

I'm getting "% Complete: %60" for a tooltip on a partially completed task.

Comment 18

[gekacheka]

I get "% Complete: 60%", not clear to me why yours would be different.

Comment 19

[Mostafa Hosseini]

(In reply to comment #18)
> I get "% Complete: 60%", not clear to me why yours would be different.

Yes. I'm getting the same thing. (Sorry for the misplaced % sign). I just
thought the % before Complete shouldn't be there. ( Looked wrong to me at the
first glance ). If that is the intended behaviour then that's fine.
I'll check in the patches.

Comment 20

[Mostafa Hosseini]

Comment on attachment 163357 [details] [diff] [review]
content patch: combines code from calendar.js and unifinder.js to mouseoverPreviews.js (updated)

(Took out commented out function )

Comment 21

[Mostafa Hosseini]

Patches checked in.
This was a great clean up.
Thanks gekacheka.

Comment 22

[gekacheka]

Thanks for your persistence Mostafah.

One known problem is that I've been unable to make the description wrap, but we
can leave that for a future bug.

Comment 23

[Reed Loden [:reed]]

The bugspam monkeys have been set free and are feeding on Calendar :: Sunbird Only. Be afraid for your sanity!