Add birthday fields to address book

VERIFIED FIXED in Thunderbird 3.0a3

Status

MailNews Core
Address Book
P2
enhancement
VERIFIED FIXED
18 years ago
2 years ago

People

(Reporter: hangas, Assigned: pi)

Tracking

(Blocks: 8 bugs, {polish, uiwanted})

Trunk
Thunderbird 3.0a3
polish, uiwanted
Dependency tree / graph
Bug Flags:
blocking-aviary1.0 -
blocking-aviary1.0mac -
blocking-thunderbird3 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: ui urgent [delight])

Attachments

(8 attachments, 9 obsolete attachments)

6.96 KB, patch
(not reading, please use seth@sspitzer.org instead)
: review-
Details | Diff | Splinter Review
12.81 KB, patch
(not reading, please use seth@sspitzer.org instead)
: review-
Details | Diff | Splinter Review
34.29 KB, image/jpeg
Details
41.74 KB, patch
standard8
: review-
Details | Diff | Splinter Review
17.14 KB, image/png
Details
41.19 KB, image/png
Details
47.72 KB, patch
Bienvenu
: superreview+
clarkbw
: ui-review+
Details | Diff | Splinter Review
4.20 KB, image/png
Details
(Reporter)

Description

18 years ago
Need to add birthdate to new card/edit card and the card view.  This feature
bumped to M14 by Sol in favor of other things.
(Reporter)

Updated

18 years ago
Blocks: 10791
Status: NEW → ASSIGNED
Target Milestone: M14
(Reporter)

Comment 1

18 years ago
Marking as post PR1 with Sol's blessing.

Updated

18 years ago
QA Contact: lchiang → esther
(Reporter)

Comment 2

18 years ago
*** Bug 16335 has been marked as a duplicate of this bug. ***

Updated

18 years ago
Target Milestone: M14 → M15
(Reporter)

Updated

18 years ago
Whiteboard: ui urgent

Comment 3

17 years ago
Syncing priority with marketing.  Moving to P2 to connote "In" for beta2.
Priority: P3 → P2

Comment 4

17 years ago
move to M16.  Not M15 stoppers
Target Milestone: M15 → M16

Comment 5

17 years ago
Scott, clearing Hangas' plate for skins work.  Probably need to reassign some of 
these again.
Assignee: hangas → putterman
Status: ASSIGNED → NEW

Comment 6

17 years ago
Nice :-) Perhaps mozilla could prompt to send a birthday greeting as well ?

Comment 7

17 years ago
Steve, 

what's the status on this?  Do we need to push for this for nsbeta2 or can we 
move this out of M16?

Comment 8

17 years ago
Never got a pull for this, so it's out.  Marking M20.
Target Milestone: M16 → M20

Comment 9

17 years ago
Moving feature bugs to target milestone "Future" for next release consideration.
Target Milestone: M20 → Future
Looks like a fun feature, accepting QA contact for this.
QA Contact: esther → stephend
QA Contact: stephend → pmock

Comment 11

17 years ago
reassigning to chuang.
Assignee: putterman → chuang

Comment 12

17 years ago
Assign to myself.
QA Contact: pmock → fenella

Updated

16 years ago
QA Contact: fenella → nbaca
Netscape 6.1 is out already, so how are things going for this birthday field?

Comment 14

16 years ago
reassigning to racham
Assignee: chuang → racham

Comment 15

15 years ago
Will this bug be addressed in 1.0?

Comment 16

15 years ago
Hi,

I am not sure how important this is to many people, but i do see it being very
useful later on when calendar becomes a mozilla feature. 

Please Please Please add this field and i will give up my MS outlook for good! 

If this field is in the addressbook, others can write greetings etc XPIs. 

Comment 17

15 years ago
*** Bug 85344 has been marked as a duplicate of this bug. ***

Comment 18

15 years ago

*** This bug has been marked as a duplicate of 85344 ***
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → DUPLICATE

Comment 19

15 years ago
Comment #18 from Erich 'Ricky' Iseli:
 > This bug has been marked as a duplicate of 85344.

I think that marking this bug (13595 - add birthdate field in Addressbook) as a
duplicate of Bug 85344 (add more fields) is not legal:
Adding birthday field is clearly not dependent on adding AIM etc. fields.
Since there were 6 votes for this bug (13595) and only 2 votes for the more
comprehensive Bug 85344, marking this as a duplicate seems an unfair manipulation.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---

Updated

15 years ago
Blocks: 85344

Comment 20

15 years ago
Birthday Fiels are already in the addressbook, just not in the UI:
  (A0=WebPage2)(A1=BirthYear)(A2=BirthMonth)(A3=BirthDay)(A4=Custom1)
  (A5=Custom2)(A6=Custom3)(A7=Custom4)(A8=Notes)(A9=LastModifiedDate)

Comment 21

15 years ago
I think that a birthday reminder would be very useful. :-) I'd use it! I'm 
terrible at remembering to check my written calendar until after the date.

Comment 22

15 years ago
Yes, this would be a very useful feature, especially once integrated with the
calendar.

Comment 23

15 years ago
*** Bug 152252 has been marked as a duplicate of this bug. ***

Updated

14 years ago
Severity: normal → enhancement
Summary: [Feature] Add birthday fields to address book → Add birthday fields to address book
mass re-assign.
Assignee: racham → sspitzer
Status: REOPENED → NEW

Comment 25

14 years ago
I decided to take a look around the source to see how quickly this could be
implemented (as part of the main tree or as an extension) and I have it working
on my home copy - nsIAbCard.idl already declares a "birthDay" "birthMonth" and
"birthYear" which can be written to and read back from as part of a cardproperty. 

I've never worked with patching the main code, only with extensions, but I'm
willing to take it upon myself to get a patch to show these fields, if no one
else is currently working on it. If this isn't wanted in the main build, I'd be
willing to write an extension to implement it.

Cheers
Mike Krieger

Comment 26

14 years ago
Created attachment 128098 [details] [diff] [review]
Adds Birthday to Card Edit and Card View

First time with a patch so if there is something wrong with the formatting or
the code please let me know.

Updated

14 years ago
Attachment #128098 - Flags: review?(sspitzer)

Comment 27

14 years ago
This looks like a trivial change to implement this fix. Bumping it up on the radar.
Keywords: mail6, polish

Updated

14 years ago
Blocks: 151994

Comment 28

14 years ago
It would be great a new tab called "Personal" and integrate inside the fields
corresponding to the personal data ("phone", "physical address", the new one
"birth date"...)

Comment 29

14 years ago
I agree

Comment 30

14 years ago
Created attachment 132079 [details] [diff] [review]
Add Birthday, Anniversary, Spouse, and Family (Maiden) Name fields

This patch adds more than what's asked by the bug, but all changes are of a
similar format.  With patch, the Other tab includes Birthday and Anniversary
boxes, with spouse and family name fields in the anniversary box.  In the AB
cardview pane, fields show up under Birthday and Anniversary headings, despite
being able to list spouse/family without an anniversary date attached.

Updated

14 years ago
Attachment #132079 - Flags: review?(scott)

Comment 31

14 years ago
Other notes:  The spacing/tabbing is awkward, but roughly consistent with the
source files and the previous patch (on which this was based).  Once this is
checked in I'll be happy to move certain contents into a personal tab, as
mentioned in comment 28.
The fix sounds great!
Would it be possible to have the spouse act as some sort of link, to another card?
eg. I have to cards; 'mom' and 'dad' with the link feature, I would only have to
create the Anniversary for one of them, the other would get updated automatically.

Comment 33

14 years ago
When is the going to be fixed for the general public

Comment 34

14 years ago
Created attachment 136735 [details] [diff] [review]
Birthday field et al, v2

Addressed some comments by doron on IRC.  Spacing fixed; unnecessary width &
flex statements removed.  Mike Krieger, would you look at this patch and offer
suggestions also?

Updated

14 years ago
Attachment #132079 - Attachment is obsolete: true

Comment 35

14 years ago
Looks good; the Address Book should support birthdates. Not being able to keep
track of birthdays may seriously deter a lot of people.

Updated

14 years ago
Attachment #132079 - Flags: review?(mscott)

Updated

14 years ago
Attachment #136735 - Flags: review?(neil.parkwaycc.co.uk)

Comment 36

13 years ago
Come on, for someone familiar with developing Mozilla apps it can't be too hard
to integrate a simple UI field for the already existing birthday fields in the
address book. 1.8 would be a great release for integrating this feature. Anyone?
thanks for starting the UI side of this fix.

I think this should go in a "personal" tab, like OE6 does it.  (I'll attach a
screen shot of OE6 for reference.)

also

1)  is having attributes for day, month, year the right way to do this?
2)  var formattedBirthday = card.birthDay + "/" + card.birthMonth + "/" +
card.birthYear;

that format should live in a properties file.  for example, in the US that would be:

"mm/dd/yyyy", but other places my be "dd.mm.yyyy"

or better yet, we could store this a card.birthDay as a time value, and use the
existing interfaces to format / parse.  

I obviously haven't thought this through yet.

we should also worry about:

1) ldif import / export
2) txt,tab,csv import / export
3) import from other mailers (eudora win, eudora mac, outlook express, outlook)
4) ldap attribute matching
5) mapping for nsAbOutlookAddressbook code, nsMapiAddressbook, nsWabAddressbook...
6) hidding these fields when editing a vCard

after investigating the ldap attribute / import / export / outlook / mapi / wab
issues might lead us to decide to have three fields for birthday, or just one.
Created attachment 149688 [details]
screen shot of oe6 personal tab
let the record show my regression was a world class screw up.

unfortunately,
ftp://ftp.mozilla.org/pub/mozilla.org/mozilla/nightly/latest-trunk/ has 5/24 as
the latest linux builds, so the dups will keep coming until there is a newer build.

fortunately, there is a 5/30 linux build in the lastest 1.7 folder.
also, what's the inspiration for the "maiden name" field?  

do other popular addressbook have that field?  I'm not sure about that one.
Comment on attachment 136735 [details] [diff] [review]
Birthday field et al, v2

not ready yet
Attachment #136735 - Flags: review?(neil.parkwaycc.co.uk) → review-
Comment on attachment 128098 [details] [diff] [review]
Adds Birthday to Card Edit and Card View

not ready yet
Attachment #128098 - Flags: review?(sspitzer) → review-
since we are talking about dates, see
http://lxr.mozilla.org/mozilla/source/mailnews/base/resources/content/dateFormat.js
and
http://lxr.mozilla.org/mozilla/source/mailnews/base/resources/content/markByDate.js

we should be able to leverage the code in dateFormat.js

Comment 44

13 years ago
Seth, thanks for putting your effort on this.
Why should we be hiding the birthday fields when editing a vCard (#6 on your
to-do list)? Doesn't the vCard standard allow birthday fields?
As for the date/time format, I think your idea is very good that we use the
existing system setting for this.

Comment 45

13 years ago
For the maiden name, well I can just think of the scenario when someone you know
gets married and you still use the old name as a habit. Other than that, IMHO it
would only be interesting for insurance/medical-like purposes.

Comment 46

13 years ago
Seth, a quick note regarding the formatting of the dates (three separate fields):
The backend is set up currently to handle those fields as they're shown, so the
patches attached so far were, as you mentioned, basically just UI to expose the
capability.  That's also the reason for the maiden name field -- just hooking up
all the fields that existed but weren't being used.  IIRC, it's called
"familyName" in the code, but maiden name made more sense to me.
It probably would be good to store the date info differently as you suggest, and
perhaps use a datepicker like what's found in the calendar project.  But such
work is beyond what I can do, so I won't be able to provide future patches along
those lines.

Comment 47

13 years ago
(In reply to comment #37) 
> 1) ldif import / export
> 2) txt,tab,csv import / export
> 3) import from other mailers (eudora win, eudora mac, outlook express, outlook)
> 4) ldap attribute matching
> 5) mapping for nsAbOutlookAddressbook code, nsMapiAddressbook, nsWabAddressbook...
> 6) hidding these fields when editing a vCard
It seems that another point should be added :
7) printing of cards and address books
*** Bug 240519 has been marked as a duplicate of this bug. ***

Updated

13 years ago
Flags: blocking-aviary1.0mac?
Flags: blocking-aviary1.0?

Updated

13 years ago
Flags: blocking-aviary1.0mac?
Flags: blocking-aviary1.0mac-
Flags: blocking-aviary1.0?
Flags: blocking-aviary1.0-

Comment 49

13 years ago
(In reply to comment #20)
> Birthday Fiels are already in the addressbook, just not in the UI:
>   (A0=WebPage2)(A1=BirthYear)(A2=BirthMonth)(A3=BirthDay)(A4=Custom1)
>   (A5=Custom2)(A6=Custom3)(A7=Custom4)(A8=Notes)(A9=LastModifiedDate)
> 

So, how does one access or use this information. I was in the process of
switching my address book from Outlook to TB when I realized I could no longer
track birthdate. This is a *MAJOR* draw back.

Comment 50

13 years ago
I consider this bug critical when releasing version 1.0 of Thunderbird.
Thunderbird 1.0 without a birthday field in its address book is a major drawback
(and I'm sure that this lack will be mentioned by some journalists when
reviewing thunderbird - something the thunderbird development team might
consider). Especially for Outlook-Switchers this could be a real showstopper.

Comment 51

13 years ago
This bug is really release critical for Thunderbird 1.0. I believe so, too.
There is a patch waiting here and nothing happens. Why?
This bug was entered in 1999. Is that true? I can't believe it.
I hope for a birthday field (and later on a connection to Sunbird)  

Comment 52

13 years ago
gleppert@gmx.de: thank you for volunteering, please address comment 37 and post
your patch asap.
Assignee: sspitzer → gleppert
Product: Browser → Seamonkey

Comment 53

13 years ago
Oh, I guess there was a misunderstanding. I don't have a patch waiting here at
home. I meant the existing patch "Birthday field et al, v2" from 2003-12-03.
What did happen to this patch? Unfortunately I'm not able to write a patch
myself. I hope someone will do it.
Gerald

Comment 54

13 years ago
I nearly managed to migrate my father to Thunderbird + Calendar, but he needed
to display the birthdays on the calendar.... so he still uses Outlook for now...

Comment 55

13 years ago
I'll take this bug, but I need Gerald to unassign himself from the "Assigned To:
" field (or have someone do it for him, as it's pretty clear he did not mean to 
assign himself to this bug). I did the original patch, so I'll build on that and 
the second patch and address Seth's comments in Comment #37. No estimate on 
completion date, but I'll at least resume work on a bug that seems to have 
stalled. If any new ideas have come up regarding implementation of this, please 
let me know.

Updated

13 years ago
Assignee: gleppert → mikekrieger

Comment 56

12 years ago
I'm looking forward that this project becomes a new etxension.

I wish, the birthday field in the address book as proposed here would integrate 
in the calendar extension (confer Lostus Organizer, if you know it)

Comment 57

12 years ago
Hi

since I've seen no movement on this bug, I descided to Fix this bug by myself. 

This is my first patch ;-), I never contributed before. After 7 hours of trying
I finally learned enough to hack Mozilla. 

Thus I've no clue how to make a Patch File. I've packed the Changed files into a
rar file. Simply decompress it and copy it into your chrome directory.

This "Patch" is based on the previous one.

For formating the dates, I reused the dateUtils.js from the Calendar project. I
simply copied it content\messenger\addressbook\. But this code needs a property
from calender, so I copied the needed file to
content\messenger\addressbook\locale and updated the code. It is definitely the
wrong place for the property File, but I found no better one. "dateUtils.js" has
now primitive out of bound checks, this might also be usefull for the calendar
project. 

The Birthday and Anniversary Fields are now fully functional, only the UI is
misaligned. 

Could some one please review the code? 

Thanks

Comment 58

12 years ago
Created attachment 180747 [details]
Possible Patch

Updated

12 years ago
Attachment #180747 - Flags: review?

Comment 59

12 years ago
Created attachment 180798 [details] [diff] [review]
real Patch instead of an Rar file
Attachment #180747 - Attachment is obsolete: true
Attachment #180798 - Flags: review?(mscott)

Comment 60

12 years ago
schmid-thomas@gmx.net: this bug is currently in the "mozilla applcation suite"
product, but your patch is to thunderbird, i don't suppose i could convince you
to provide a similar patch to the suite? :)

Updated

12 years ago
Attachment #180747 - Flags: review?

Updated

12 years ago
Blocks: 163993

Updated

12 years ago
No longer blocks: 163993

Comment 61

12 years ago
If the patch is for thunderbird, and the bug is for the suite, does that mean
the patch will never make it in? On the other hand, I would have thought they
are sharing the adress book code?

I would love to see that birthday field soon!
Component: Address Book → MailNews: Address Book
Product: Mozilla Application Suite → Core
QA Contact: nbaca → addressbook
Since Thomas is working on this bug (enhancement), maybe it should be marked as ASSIGNED rather than NEW, no?

In one of the comments, there's mention about hidding the birthday field from vCard.  But why?  vCard has a field for birthday.
http://www.faqs.org/rfcs/rfc2426.html
Section 3.1.5

Should this bug be moved from "Core" product to "Thunderbird" to be more appropriate?

And yes, I think OE is a good example for TB to copy interface and functionalities.  OE has copied a lot from NS, so I don't see why TB can't copy back from OE.
Comment on attachment 180798 [details] [diff] [review]
real Patch instead of an Rar file

Sorry, but I don't think we can accept this patch as it is.

The date formatting code needs to be moved to a central place so that we're not duplicating calendar's code. Additionally, IMO we should really be using a date picker like calendar does as well - the moving of this to a core place (i.e. toolkit) is already contained within bug 92174.

Also, the patch also doesn't apply to the current source tree - we have made improvements to the new/edit card dialogs since this patch was posted.

Please do feel free to submit a new patch though, but obviously we'll need to push bug 92174 as well.
Attachment #180798 - Flags: review?(mscott) → review-
Depends on: 92174

Comment 64

12 years ago
Please keep in mind that it is not uncommon to store a partial birthdate. I frequently know the month and day, e.g., but not always the year.

Because of this, I think copying OE by adding a date-picker widget would be suboptimal. Date-pickers are also typically very irritating and inefficient when choosing a year several decades from the initial value.

Comment 65

12 years ago
Outlook manages it quite well in that you can type dates but also use the date picker.  This means you can jump very quickly to the right year by typing that in (so for example I could type 1957 into the text box and click down and it goes to January 1957 in the date picker).
*** Bug 323796 has been marked as a duplicate of this bug. ***

Comment 67

11 years ago
(In reply to comment #1)
the road is long since 1999, but this querry still remain... I don't think it will be treated in Thunderbird 2.0, but is there a plan to do it one day? It will be a great improvement in the management of personnals contacts.

(In reply to comment #67)
> (In reply to comment #1)
> the road is long since 1999, but this querry still remain... I don't think it
> will be treated in Thunderbird 2.0, but is there a plan to do it one day? It
> will be a great improvement in the management of personnals contacts.
> 
Given that bug 92174 has now got some way to implementing a Date picker that we will probably be able to use, then it is much more likely. It definitely won't be for Thunderbird 2.0/SeaMonkey 1.1, but Thunderbird 3.0 and SeaMonkey 1.5 are definite possibilities.

Comment 69

11 years ago
I'd love to see a birthdate field being added as well, it's is a simple but valuable option. In addition to this I think it should have the option to add that date to the calendar automatically but that's probably a lightning/sunbird thing to implement. Doesn't hurt to keep it in mind though ;-)

Comment 70

11 years ago
By the way, shouldn't this bug be listed under the 'Thunderbird' product instead of 'Core'?
(In reply to comment #70)
> By the way, shouldn't this bug be listed under the 'Thunderbird' product
> instead of 'Core'?

No. It affects both SeaMonkey as well as Thunderbird address books and should be resolved as a Core Address Book problem.

Comment 72

11 years ago
(In reply to comment #69)
> I'd love to see a birthdate field being added as well, it's is a simple but
> valuable option. In addition to this I think it should have the option to add
> that date to the calendar automatically but that's probably a lightning/sunbird
> thing to implement. Doesn't hurt to keep it in mind though ;-)

That's bug #151994. Blocked, of course, by this one. :-)

Cheers,
Dominik
In response to some comments: I would also vote for three GUI fields than a single field.
Furthermore, is it possible not to specify the year?  You know, women's age is a secret ;)  Seriously, sometimes I just know the month and day of my friends without knowing the year. So, I just put in fake years like 1900!!  It would be nice to be able to put "unspecified" in year.  But the datepicker wouldn't make sense in this case.  Maybe a generic date picker could be used instead?  A picker which doesn't care about day of the week.

Actually, personally, I would prefer a date picker which only displays month and day without shifting the days to match the week. Well, who would really know or care on what day of the week one's friend is born??

Comment 74

11 years ago
*** Bug 351621 has been marked as a duplicate of this bug. ***
Please add keyword uiwanted since it appears that it's holding on an acceptable date control.

Updated

11 years ago
Keywords: uiwanted

Comment 76

10 years ago
It's been quite on the front of the birthday date for some time now. Since this is for me one of the important bugs, I would like to put it back on the radar. Unfortunately I can't code myself or I would have really worked on this one...

Does anybody know the status, and when it will make it to Thunderbird/Lightning. 

I really hope this will make it very soon. Thanks all for making an effort on it!

Comment 77

10 years ago
There is an extension morecols for adding birthdays to the adressbook (The ui could use some improvements):
https://nic-nac-project.org/~kaosmos/morecols-en.html

and some days ago ThunderBirthDay 0.2 (for lightning) got released:
http://forums.mozillazine.org/viewtopic.php?t=580684
https://addons.mozilla.org/en-US/thunderbird/addon/5337

tested it and works like a charm, showing the birthdays in lightning.

Comment 78

10 years ago
I did so too and it woorks great, however, I think Thunderbird has come to a point, were it really needs a mature address book. Don't get me wrong the add-on works great, but I would really like to see this solved natively.

So I'm still hoping.....

Patrick

Comment 79

10 years ago
A good address book should have a birthday-field in standard (without add-on).
There are some other bugs, about adding some other fields. I think the
birthday-field should also be implemented in Thunderbird and also in
Lightning/Sunbird without add-on. The birthday field should also be availible
as complete date-field (not only birthyear), which can be displayed in the
address book list.

The missing birthday-field (and other fields) prevent me to use Thunderbird for
mailing and I think I am not the only one who wants a better address book.
Duplicate of this bug: 401379

Updated

10 years ago
Flags: blocking-thunderbird3?

Comment 81

10 years ago
Like Thomas, I really miss birthday-field in standard in the address book.

Updated

9 years ago
Flags: wanted-thunderbird3+
Flags: blocking-thunderbird3?
Flags: blocking-thunderbird3-
Blocks: 423488
Re-nominating per-discussion post Thunderbird status meeting - we really want this for TB 3.0.
Flags: blocking-thunderbird3- → blocking-thunderbird3?
Changing from wanted+ to blocking+.  I assert that this is the epitome of the kind of "low hanging fruit" changes that we want for Tb3: it's something a lot of people want, and it's very little work.
Flags: wanted-thunderbird3+ → blocking-thunderbird3.0a1+
blocking 3.0a1 was a mistake; fixing to blocking tb3.
Flags: blocking-thunderbird3?
Flags: blocking-thunderbird3.0a1+
Flags: blocking-thunderbird3+
Product: Core → MailNews Core
pi has offered to look into this, reassigning to him.
Assignee: mikekrieger → joshgeenen+bugzilla
(Assignee)

Comment 86

9 years ago
Created attachment 334386 [details] [diff] [review]
Work in progress

This patch isn't the final one for which I will request r and sr; it is just to get some feedback.

So far, it adds a popup datepicker [1] to the top of the Other tab.  The tabs will likely change before Thunderbird 3 is released so it will probably reside in a more appropriate tab in the future (my guess would be a "Personal" tab).  It only saves the date if is is different than the date on which the dialog was opened.  My only question is, how can the user remove the birthday once it is set?  Should there be a button?

It shows the birthday in the format generated by Date.toLocaleDateString [2], which varies by platform and locale, in the card view.

Finally, it adds BirthMonth (as an integer from 0 - 11) and BirthDay to LDIF imports and exports and I updated the LDIF import unit test.  BirthYear was already supported for all text imports and exports, and CSV/tab imports and exports already worked for BirthYear, BirthMonth, and BirthDay.

The patch requires strings that it adds to the en-US locale and I haven't tried building Sea Monkey with it yet, and I'm not sure why this line appears since I looked at the source and the line is still there:
-  // get phonetic fields if exist

There is a screenshot here (Linux, en-US) [3]

[1] http://developer.mozilla.org/en/docs/XUL:datepicker
[2] http://developer.mozilla.org/en/docs/Core_JavaScript_1.5_Reference:Global_Objects:Date:toLocaleDateString
[3] http://www.pirules.net/other/img/birthday_01.png

Comment 87

9 years ago
Created attachment 334395 [details]
MoreFunctionsFor Abook UI solution

Another solution to display of Birthday date in one of two added tabs. This screen capture is with the MoreFunctionsForAddressbook extension installed.
My birthday is totally in 1592. :-)

Anyways, if anyone wants to see this in action, I'll be applying the patches to my mercurial comm-central clone, <http://hg.mozilla.org/users/Pidgeot18_gmail.com/ab_rewrite/>.

In terms of UI, I prefer the datepicker over the three-field approach, but I haven't played around with whether I prefer popup or grid.
Status: NEW → ASSIGNED

Comment 89

9 years ago
The Picker is a nice tool and if it were a Widget it would be available for use with Lightning and other future uses.  

Comment 90

9 years ago
Comment on attachment 334386 [details] [diff] [review]
Work in progress

>+            <label control="BirthdayPopup" value="&Birthday.label;" accesskey="&Birthday.accesskey;" class="CardEditLabel"/>
>+            <datepicker type="popup" id="birthday"/>
shouldn't this be <label control="birthday"?
Just going to add some notes after chatting w/ pi on IRC. 

Often people don't know the year someone else was born and it's not actually necessary information.  Of course it's probably possible for the user to guess or calculate the year out, but this is a step that shouldn't be necessary.  A main goal of the birthday field is so we can make the user aware of birthdays from now on.  So while it'd be nice to have the correct year, I don't see this as required information.

This makes the birthday picker a bit different than a normal date picker (i.e. it's not as common to enter a birthday for today or near today's exact date as it will be for dates a while in the past).   And since the date picker makes you scroll through dates month by month it can be a bit difficult to get to the right day/month/year.

So it'd be nice to see a birthday picker that only requires the day / month and takes the year as optional information.  If we can use the datepicker for choosing the day and month only, then allow people to tab over to the year entry for entering a year if they choose to.

A chooser that looks similar to this:
[ DD / MM |v ]  [ YYYY ]

One slight problem this can create is that you might not be able to choose a leap year day very easily.  We could just default to the last leap year as the initial year in the datepicker such that all days are visible in that year.

There could also be a storage issue of saving a real date to the addressbook and then later the editor not knowing that they year returned wasn't chosen by the user.  If this is a problem we should store an additional flag to indicate that they year wasn't set by the person.

Comment 92

9 years ago
Can't we have an option either to set the Day & Month or to put their age in that year. For instance many people might now that it's somebody's 40th birthday in the coming year, but not know what year that it means they were born. I think, personally, this is the more likely scenario.

Comment 93

9 years ago
I'm not sure what the benefit of putting partial dates in the birthday field are. It seems like it would only serve to allow invalid dates to the field, requiring far more error-checking when doing anything useful with the data (showing birthdays on a calendar, i.e.).

It seems reasonable, to me anyway, to put partial dates in the Notes field.

GRAMPS had to do quite a bit of extra work to handle partial dates: http://gramps-project.org/wiki/index.php?title=Gramps_3.0_Wiki_Manual_-_Entering_and_Editing_Data:_Detailed#Editing_Dates

That seems like a great deal of overhead. If partial dates *are* supported, maybe some code/algorithm reuse (or at least syntax consistency) from GRAMPS can be done?
I dont think that a partial date is a good solution. One thing to keep in mind is the synchronization capability of the adress book with other device... . This functionality become more and more popular.
As birthday field is something that could be synchronize, it need to respect the standard coding for birtday of a vcard (as it is the most common way to sync data with another device). As it seems that it need a year information (see: http://en.wikipedia.org/wiki/VCard#VCard_Properties ), I think we should only allow a standard date, with the year.
It's up to the user that don't know all the information of the birthday to consider if he wants to use the note field instead of the birtday field.

Comment 95

9 years ago
A birthday reminder would not work if the birthdate is in the note field. Following the standard conflicts with real life and results in users doing ugly workarounds (e.g. using the year 9999 for unknown birthyear).

Comment 96

9 years ago
(In reply to comment #94)
> I dont think that a partial date is a good solution. One thing to keep in mind
> is the synchronization capability of the adress book with other device... .
> This functionality become more and more popular.

There are literally scores of people in my address book for whom I know (or can look up) the day and month of their birthday, but not the year / their age.
I synchronize that address book... exactly never.

Of course, I'm an "anecdote" - but I suspect that if any "data" was available, it'd show that the vast majority of people are in the exact same situation.

When synchronizing, it could simply leave out such birthdays as don't have a valid year set (maybe with some low-key warning?); but I really don't think that use-case should be important enough to toss out the ability to record birthdays without year altogether!

Comment 97

9 years ago
Created attachment 335298 [details]
zimbra webclient birthday datepicker

Comment 98

9 years ago
Exploring ideas around sync of partial dates.

1. use a fake year to help sync a Thunderbird partial date.
   eg: Thunderbird dd/mm syncs remote as dd/mm/0000

2. this approach needs an option to enable/disable sync
   of birthday field because:
   (a) some users (comment #95) don't want this ugly hack and
   (b) a year of 0000 (or 9999) might trigger undesirable
       behaviour in remote clients - for example if they use
       a convential datepicker (see attached zimbra web client UI)

3. option to sync birthday field may have to default to OFF
   because of the risk of causing instability in remote clients.

4. Simplifing UI interaction here may complicate it elsewhere.

5. It is after all a birthDAY field, not a birthDATE field.
(In reply to comment #92)
> Can't we have an option either to set the Day & Month or to put their age in
> that year. For instance many people might now that it's somebody's 40th
> birthday in the coming year, but not know what year that it means they were
> born. I think, personally, this is the more likely scenario.

I think it could be really interesting and helpful to have an age calculation.  I'm not sure what the best way to enable this would be, perhaps something like this.

[ DD / MM | v ] on [ YYYY ] : [   ] age

If a person entered the year or age we could calculate the opposite value.

The partial date of Month and Day is enough information to display a person's birthday on your calendar, the year information isn't necessary for this kind of display and therefore doesn't make sense to be required.

We need to respect standards where it makes sense, however our responsibility is to our users first.  Obviously our users are also people who sync their addresbook, so valid dates is a goal for us as well.  Hopefully there is some way to get our users a birthday field that is usable and at the same time works with address book synchronization.
Whiteboard: ui urgent → ui urgent [delight]
(Assignee)

Comment 100

9 years ago
Created attachment 337224 [details] [diff] [review]
Work in progress 2

This patch demonstrates how the age/year calculation mentioned by Bryan Clark could look.  It does everything the last patch does, but now accepts the day and month only OR the day, month, and year--it doesn't accept only the year/age.

If only the day and month are given, it looks like this:
http://pirules.net/other/img/birthday_without_year.png

Otherwise:
http://pirules.net/other/img/birthday_with_year.png

The appearance of the screenshots and order of the day, month, and year will vary based on the locale and OS.

My only concern about this patch is the additional complexity in the UI, the year/age calculation, and in synchronizing.

Currently, the only way to remove a saved birthday is to set the year to zero and set the month/day to the day on which the new/edit card dialog was opened.  Also, it doesn't save the birthday if it is set to the day on which the dialog was opened and the year is 0.
Attachment #334386 - Attachment is obsolete: true

Comment 101

9 years ago
Nice bit of UI, better looking than the result from MoreFuctionsForAddressbook extension. Greatest plus is the age computation.

Comment 102

9 years ago
(In reply to comment #99)
> 
> [ DD / MM | v ] on [ YYYY ] : [   ] age
> 
> If a person entered the year or age we could calculate the opposite value.
> 

I don't think its a good idea to provide a way to specify the age. Just the year is sufficient. Being able to set the age makes the UI more complex and it's overkill to have two ways to get the same result (for such a basic item). 
The year is always the same, the age will differ. It feels wrong to have a user entered value change depending on the moment the details are being displayed. It could even result in people getting the impression that they would have to change the age manually each year. It is, however, nice to have the age displayed behind/near the date when the year is provided.
Just to note that I was talking to pi yesterday on IRC and recommended the following changes.

Use the empty text for filling in the meaning of the year and age, with an "or" separator between the two.  Deleting the text fields should remove the value for them.  We need to investigate how to do this with the day/month field.  If a person deleted the day/month field then we'd delete the year/age fields as well since they don't make sense to keep around.

I removed the spinners from the entries because I think it looks a lot simpler and cleaner as well as I tend to think that it's easier for people to enter the correct number than attempt to spin for it.

http://clarkbw.net/designs/birthday-field/birthday_with_year_new.png
(Assignee)

Comment 104

9 years ago
> Use the empty text for filling in the meaning of the year and age, with an "or"
> separator between the two.
Done.

> Deleting the text fields should remove the value for them.
> We need to investigate how to do this with the day/month field.
I was able to do this by modifying the _constrainValue and _setFieldValue functions.  However, this complicates things (the value of the datepicker's dateField and monthField are null, but the dateValue of the datepicker isn't) and might be confusing for people trying to extend the new/edit card dialogs.  It does make it easier for the user since there is no initial value and they can delete the present values to remove the properties.

> If a person deleted the day/month field then we'd delete the year/age fields as
> well since they don't make sense to keep around.
Done

> I removed the spinners from the entries because I think it looks a lot simpler
> and cleaner as well as I tend to think that it's easier for people to enter the
> correct number than attempt to spin for it.
True, I removed them.  For the datepicker, I just collapsed the element, but I changed the type of the year/age textboxes to a normal to remove the spinners and show the empty text.  If an invalid value is typed the year and age turn to null.

http://pirules.net/other/img/new_birthday.png

Comment 105

9 years ago
(In reply to comment #104)
> > If a person deleted the day/month field then we'd delete the year/age fields as
> > well since they don't make sense to keep around.
> Done

What if someone actually does only know the year or age? 

> http://pirules.net/other/img/new_birthday.png

Many users will not know their "locale settings", so it seems necessary to put "dd" and "mm" in gray letters into the day and month areas:

   Birthday: [dd.mm] [Year] or [Age]
or
   Birthday: [mm/dd] [Year] or [Age]
or
   Birthday: [mm-dd] [Year] or [Age]   <--- ISO 8601
or
   Birthday: [Year] [mm-dd] or [Age]   <--- ISO 8601

http://www.iso.org/iso/support/faqs/faqs_widely_used_standards/widely_used_standards_other/date_and_time_format.htm/#how-it-works
(Assignee)

Comment 106

9 years ago
(In reply to comment #105)
> (In reply to comment #104)
> > > If a person deleted the day/month field then we'd delete the year/age fields as
> > > well since they don't make sense to keep around.
> > Done
> 
> What if someone actually does only know the year or age? 
Un-done for now.

I tried to do that, but it isn't possible to set the empty text in a datepicker (or a number textbox).  In the next patch (that I will attach as soon as I make sure suite compiles with it), the label will be changed to:
Birthday (MM/DD):
(Assignee)

Comment 107

9 years ago
Created attachment 338574 [details] [diff] [review]
Patch v1

Here is the first patch I'll submit for review.

-Adds a modified datepicker that allows the user to type or pick a date from a popup.  Only the month and day are shown in the datepicker, and the month/day value can be removed.
-Also adds a year field with an age textbox.  Entering a value in one calculates the complementary value and updates the datepicker's year.  Changing the year in the datepicker changes the year (and age) textboxes.
-Adds BirthMonth and BirthDay to LDIF imports and exports.
-Displays the birthday in the card view pane if the month and day and/or year are present.

It does not do anything with vCards or Lightning and doesn't add anything to the treecols for the address book results pane.  Adding a treecol for one particular piece of a birthday (like the BirthYear) wouldn't be difficult if somebody would like me to do so.
Attachment #337224 - Attachment is obsolete: true
Attachment #338574 - Flags: ui-review?(clarkbw)
Attachment #338574 - Flags: review?(bugzilla)

Comment 108

9 years ago
Comment on attachment 338574 [details] [diff] [review]
Patch v1

> +<!ENTITY Birthday.label                 "Birthday (MM/DD):">

I don't think this is at all acceptable, we're plenty of people using en-US builds but would never enter a date that way.

Comment 109

9 years ago
> (From update of attachment 338574 [details] [diff] [review])
> > +<!ENTITY Birthday.label                 "Birthday (MM/DD):">
                                                        ^^^^^
Is that text hard-coded and does it not change with the users' locale setting? Yikes! If so, then please at least don't use the *worst* way of showing the date (mm/dd yyyy <-- the order is illogical).

BTW: We have gray text in the year and age fields (...[Year] or [Age]). Is everyone here sure there is no way to put gray text into the day and month field?

BTW: Does the patch require the user to enter four digits, even if the date contains single digits (e.g., 1st of March --> 1.3. vs. 01.03.)? Must the user enter the second period? CAN the user enter the second period? (Best answers would be: no and yes)
(Assignee)

Comment 110

9 years ago
Comment on attachment 338574 [details] [diff] [review]
Patch v1

Removing review requests as I found a bug while entering a date manually.
Attachment #338574 - Flags: ui-review?(clarkbw)
Attachment #338574 - Flags: review?(bugzilla)
(Assignee)

Comment 111

9 years ago
I apologize if this looks hastily written; I am between classes and don't have much time.

(In reply to comment #108)
> (From update of attachment 338574 [details] [diff] [review])
> > +<!ENTITY Birthday.label                 "Birthday (MM/DD):">
> 
> I don't think this is at all acceptable, we're plenty of people using en-US
> builds but would never enter a date that way.
I don't quite understand what you are saying with the "we're plenty of people using en-US builds...", but if I enter a date in something localized for the US (application, form, paperwork, etc.), it will be in the MM/DD/YYYY OR MM-DD-YYYY format nearly every time.  People in the US expect that format even though it doesn't make much sense IMO.

(In reply to comment #109)
> > (From update of attachment 338574 [details] [diff] [review] [details])
> > > +<!ENTITY Birthday.label                 "Birthday (MM/DD):">
>                                                         ^^^^^
> Is that text hard-coded and does it not change with the users' locale setting?
> Yikes! If so, then please at least don't use the *worst* way of showing the
> date (mm/dd yyyy <-- the order is illogical).
The text isn't hard-coded.  ENTITY values like this one are localized, so the localizer could change it based on how it will appear to the user.  mm/dd yyyy is illogical, but that's how dates are formatted in the US for the most part.

The order of the elements: datepicker, year, age is hardcoded, but the order of the month/day in the datepicker changes.

> BTW: We have gray text in the year and age fields (...[Year] or [Age]). Is
> everyone here sure there is no way to put gray text into the day and month
> field?
This isn't directed at me, but I tried and don't see an easy way to do it.  Setting the emptytext of a datepicker has absolutely no effect.  It could possibly be done with heavy modifications to the datepicker, but at a certain point it would be far easier to write a new datepicker.  If somebody knows how to do it, please let me know.  It could be changed in a future patch or bug.

> BTW: Does the patch require the user to enter four digits, even if the date
> contains single digits (e.g., 1st of March --> 1.3. vs. 01.03.)?
No, but the datepicker is split into 2 fields: day and month (order varies).  You can type 1 <tab> 1 and the 1 s will change into 01 in en-US, but not necessarily other locales.

> Must the user
> enter the second period? CAN the user enter the second period? (Best answers
> would be: no and yes)
Short answers: No and Yes.  In my locale (en_US), a datepicker uses '/' to separate portions of a date, but the user doesn't have to type these, and there is only one, which is in the datepicker between the month and day values.  If you try to type a period, slash, or anything but a number in a datepicker it will be ignored, so the user can enter a period if they wish, but it won't show up.

(In reply to comment #110)
> (From update of attachment 338574 [details] [diff] [review])
> Removing review requests as I found a bug while entering a date manually.

I have class for the next hour, but I believe I have fixed the issue.  I'm going to test the UI more and possibly ask questions in #maildev about the last two comments after class and hopefully get a newer patch out within a few hours.

Comment 112

9 years ago
(In reply to comment #111)
> (In reply to comment #108)
> > (From update of attachment 338574 [details] [diff] [review] [details])
> > > +<!ENTITY Birthday.label                 "Birthday (MM/DD):">
> > 
> > I don't think this is at all acceptable, we're plenty of people using en-US
> > builds but would never enter a date that way.
> I don't quite understand what you are saying with the "we're plenty of people
> using en-US builds...", but if I enter a date in something localized for the US
> (application, form, paperwork, etc.), it will be in the MM/DD/YYYY OR
> MM-DD-YYYY format nearly every time.

Lots of people outside the US still use en-us Thunderbird/SeaMonkey builds. We want the US language, but not the US date formatting. All other dates in the program pick up the date format from the OS settings (which I've personally set to yyyy-MM-dd). The birthday field should do the same.
(Assignee)

Comment 113

9 years ago
(In reply to comment #112)
> Lots of people outside the US still use en-us Thunderbird/SeaMonkey builds. We
> want the US language, but not the US date formatting. All other dates in the
> program pick up the date format from the OS settings (which I've personally set
> to yyyy-MM-dd). The birthday field should do the same.

Based on feedback, the datepicker now contains the year as well (and can also be blank, unlike normal datepickers).  The order in which the three portions of a date (day, month, and year) appear depends on the locale settings (which I believe can vary with OS), specifically on how the output of Date.toLocaleFormat("%x") appears.  See the datepicker code for details:

http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/content/widgets/datetimepicker.xml#792

If you want to know what this is for your locale, go to Tools | Error Console in Firefox, TB, etc. and enter:
alert((new Date()).toLocaleFormat("%x"));

Sander, is the date format correct for you?

A popup will display the current date.  For me, it is 09/15/2008, as expected.

It now looks like this when blank in my locale.  The two extra gContactSync tabs are from my extension and are unrelated to this patch.
http://pirules.net/other/img/birthday.png

Any feedback on the new appearance is appreciated, and I'll submit a new patch after some feedback or before I go to bed (whichever comes first).

Comment 114

9 years ago
To help understand how sync is going to work,
it'd be helpful to have a few lines documenting what
values the BirthDay, BirthMonth and BirthYear card properties take in various circumstances.  ie
- user set a full date
- user set a day and month
- user set an age in years
- user set then unset a full or partial date
- user never set a date
(In reply to comment #108)
> > +<!ENTITY Birthday.label                 "Birthday (MM/DD):">
> I don't think this is at all acceptable, we're plenty of people using en-US
> builds but would never enter a date that way.

Magnus, do you mean having to type 04/07 instead of 4/7?  That would really be a datepicker issue and nothing to do with this bug.  Plus the datepicker does handle entering that kind of date manually.  Also it does popup a calendar chooser dialog for selecting the day / month with a mouse.

(In reply to comment #109)
> > (From update of attachment 338574 [details] [diff] [review] [details])
> > > +<!ENTITY Birthday.label                 "Birthday (MM/DD):">
>                                                         ^^^^^
> Is that text hard-coded and does it not change with the users' locale setting?
> Yikes! If so, then please at least don't use the *worst* way of showing the
> date (mm/dd yyyy <-- the order is illogical).

The DD/MM and MM/DD should changed based on locale, yes.  This is part of what the datepicker code should handle.  However the year order isn't important in this date.  For a quick human example of this you can examine person to person conversations.  People do not ask others "When is your birthday?" And then have the other person respond "1978 November 13".  Instead people shorten to the familiar (and actually important) form of "November 13" (or "13 November" depending on locale).  The extra year information is often wasted breath on the sender and wasted noise on the receiver; thus people will drop it off.

In a birthday the year isn't important in conversation because the person would have to do the age calculation in their head; because age and day/month is what is actually understandable.  Continuing the above example a common conversation would continue with "Oh, how old will you be?" with the other responding "I'll be 31".

The premise of this design (and I believe it holds true) is that people don't always know the year a person was born; but they will often know the day and month and possible the age.  If we change the order and place the year before the day and month entry we'll negate the value of this design premise.  Despite the locale preference for this, it goes against how we want people to enter the values.  For a RTL situation we would want to reverse the values.
(Assignee)

Comment 116

9 years ago
Created attachment 338801 [details] [diff] [review]
Patch v1.1

Updates the UI as displayed in the previous screenshot.

There is one known quirk that I couldn't solve right now:
When the year has been removed and the user picks a date from the datepicker, the year is automatically filled in.  This happens before an onclick listener is called.

As a sidenote, the datepicker's orientation of year, month, and day varies with locale and possibly OS.  See my previous comment for details.
Attachment #338574 - Attachment is obsolete: true

Comment 117

9 years ago
(In reply to comment #115)
> (In reply to comment #108)
> > > +<!ENTITY Birthday.label                 "Birthday (MM/DD):">
> > I don't think this is at all acceptable, we're plenty of people using en-US
> > builds but would never enter a date that way.
> 
> Magnus, do you mean having to type 04/07 instead of 4/7?  That would really be
> a datepicker issue and nothing to do with this bug.  Plus the datepicker does
> handle entering that kind of date manually.  Also it does popup a calendar
> chooser dialog for selecting the day / month with a mouse.

No, I just meant you can't hardcode the entity to include the date format (MM/DD) since that doesn't follow the system date settings which may very well differ from the locale of the build. For instance I would write 7/4, or 7.4 if we're talking about April 7, but I still want the en-US build. From the looks of the latest screenshot, the hardcoded (MM/DD) is now gone though. (good!)

Does it use the system date separator too? (./-)

Respecting leading zeros of the system date settings in the suggested values would be nice - though that can be a follow-up if it doesn't work already.
(Assignee)

Comment 118

9 years ago
Created attachment 338803 [details] [diff] [review]
Patch v1.2

The last patch is 'malformed' because I manually removed a locally edited file and accidentally removed an extra blank line before line 955.  This patch should work with newly-pulled source.
Attachment #338801 - Attachment is obsolete: true

Comment 119

9 years ago
(In reply to comment #113)
> Sander, is the date format correct for you?

Yes. Thanks! 

(I see you still fall back to a 'hardcoded' month day order if year isn't present, and am wondering if you couldn't easily retrieve the locale-based ordering to still use (to benefit day-month users), but am personally happy enough with this solution.)

Comment 120

9 years ago
(In reply to comment #113)
> http://pirules.net/other/img/birthday.png
> Any feedback on the new appearance is appreciated

Now that the year was merged into day&month, is there any visual clue that the year is not mandatory?

Could you please provide a screenshot of the date-picker?

I think the user should be informed about which order (mm/dd/yyyy or dd.mm.yyyy) the date is going to be interpreted (to avoid errors):

   Birthday: [dd.mm.yyyy] [Age]                <--- difficult but preferred
or
   Birthday (dd.mm.yyyy): [  .  .    ] [Age]


(In reply to comment #119)
> I see you still fall back to a 'hardcoded' month day order if year isn't
> present

So users entering (or selecting?) "1.4." (meaning 1st of April) will have the birthday set to 4th of January?
(Assignee)

Comment 121

9 years ago
(In reply to comment #119)
> (In reply to comment #113)
> > Sander, is the date format correct for you?
> 
> Yes. Thanks! 
> 
> (I see you still fall back to a 'hardcoded' month day order if year isn't
> present, and am wondering if you couldn't easily retrieve the locale-based
> ordering to still use (to benefit day-month users), but am personally happy
> enough with this solution.)

The birthday patch has nothing to do with the order of the month, day and year, in the datepicker.  All that code is part of the datepicker itself.  It doesn't fall back to any month/day order if the year isn't present, where are you seeing that?

Datepicker code:
http://mxr.mozilla.org/comm-central/source/calendar/resources/content/datetimepickers/datetimepickers.xml

(In reply to comment #120)
> Now that the year was merged into day&month, is there any visual clue that the
> year is not mandatory?
Other than the fact that the year is initially blank, no.

> Could you please provide a screenshot of the date-picker?

http://pirules.net/other/img/birthday.png

> 
> I think the user should be informed about which order (mm/dd/yyyy or
> dd.mm.yyyy) the date is going to be interpreted (to avoid errors):
> 
>    Birthday: [dd.mm.yyyy] [Age]                <--- difficult but preferred
> or
>    Birthday (dd.mm.yyyy): [  .  .    ] [Age]

> So users entering (or selecting?) "1.4." (meaning 1st of April) will have the
> birthday set to 4th of January?
Again, it doesn't fallback on any 'hard coded' order.  Datepickers won't work if you type the separator like that, you have to manually move to the next field.  1 <tab> 4 will change the date to whatever that value is in the current locale.  This is determined by the datepicker and not by me.  See the link above for details.
(Assignee)

Updated

9 years ago
Attachment #338803 - Flags: ui-review?(clarkbw)
Attachment #338803 - Flags: review?(bugzilla)
Comment on attachment 338803 [details] [diff] [review]
Patch v1.2

I liked what you had with attachment 338574 [details] [diff] [review].  I think we need to get that code in and fix some of the related locale issues that come up along the way.
Attachment #338803 - Flags: ui-review?(clarkbw) → ui-review-

Comment 123

9 years ago
Comment on attachment 338803 [details] [diff] [review]
Patch v1.2

a=me for SeaMonkey 2 Alpha 1 for the SeaMonkey-specific parts, given it gets reviews and actually works in SeaMonkey.
Attachment #338803 - Flags: approval-seamonkey2.0a1+
(Assignee)

Comment 124

9 years ago
Created attachment 338940 [details] [diff] [review]
Patch v1.3

This is the newest patch reverting to the old layout with a few small changes.

(In reply to comment #122)
> (From update of attachment 338803 [details] [diff] [review])
> I liked what you had with attachment 338574 [details] [diff] [review].  I think we need to get that code
> in and fix some of the related locale issues that come up along the way.
Done.  It worked with other date formats (like DD/MM/YYYY and YYYY/DD/MM) in some quick testing.
Attachment #338803 - Attachment is obsolete: true
Attachment #338940 - Flags: ui-review?(clarkbw)
Attachment #338940 - Flags: review?(bugzilla)
Attachment #338803 - Flags: review?(bugzilla)

Comment 125

9 years ago
Comment on attachment 338940 [details] [diff] [review]
Patch v1.3

this touches SM as well...
Attachment #338940 - Flags: review?(kairo)
Attachment #338940 - Flags: approval-seamonkey2.0a2?
Attachment #338940 - Flags: review?(kairo)
Attachment #338940 - Flags: approval-seamonkey2.0a2?
Attachment #338940 - Flags: approval-seamonkey2.0a1+
Comment on attachment 338940 [details] [diff] [review]
Patch v1.3

Robert has already said he's happy for this to go into SM, plus I'm an SM reviewer as well.
Comment on attachment 338940 [details] [diff] [review]
Patch v1.3

The birthday fields aren't being made read-only on cards from read-only address books. This is done in the OnLoadEditCard function - look for if (!(directory.operations & directory.opWrite)).

>diff -r 4ad411cb6f20 mail/components/addrbook/content/abCardOverlay.js
>+function calculateAge(aEvent, aElement) {
>+  var datepicker, yearElem, ageElem;
>+  if (aEvent)
>+    aElement = this;
>+  if (aElement == document.getElementById("BirthYear") ||
>+      aElement == document.getElementById("Birthday")) {
>+    datepicker = document.getElementById("Birthday");
>+    yearElem = document.getElementById("BirthYear");
>+    ageElem = document.getElementById("Age");
>+  }
>+  if (!datepicker || !yearElem || !ageElem)
>+    return;

If aElement isn't a 'BirthYear' or 'Birthday' then you could just return, as you won't have a datepicker etc...

>+function calculateYear(aEvent, aElement) {
>+  var yearElem, datepicker;
>+  if (aEvent)
>+    aElement = this;
>+  if (aElement == document.getElementById("Age")) {
>+    yearElem = document.getElementById("BirthYear");
>+    datepicker = document.getElementById("Birthday");
>+  }
>+  if (!yearElem || !datepicker)
>+    return;

Ditto here.
Comment on attachment 338940 [details] [diff] [review]
Patch v1.3

Per our discussion on irc, please make the month storage to be based on 1 - 12, similar to the day. This will then mean we work with users that have previously used MoreColsForAddressbook, and it will also provide consistency within our data storage (even though js is a bit strange).

r=me with that and the previous comments fixed.
Attachment #338940 - Flags: superreview?(bienvenu)
Attachment #338940 - Flags: review?(bugzilla)
Attachment #338940 - Flags: review+
(In reply to comment #127)
> >+  if (!datepicker || !yearElem || !ageElem)
> >+    return;
> 
> If aElement isn't a 'BirthYear' or 'Birthday' then you could just return, as
> you won't have a datepicker etc...

As per discussion on irc, this is in support of future work, so just add a comment.
(Assignee)

Comment 130

9 years ago
Created attachment 338984 [details] [diff] [review]
Patch v1.4

This fixes some issues with patch 1.3, most notably the year not updating if the datepicker's popup was set to 2000 and January appearing as '001' in the month field, and addresses the comments below.

(In reply to comment #128)
> Per our discussion on irc, please make the month storage to be based on 1 - 12,
> similar to the day. This will then mean we work with users that have previously
> used MoreColsForAddressbook, and it will also provide consistency within our
> data storage (even though js is a bit strange).
> 
> r=me with that and the previous comments fixed.
Done.

(In reply to comment #127)
> (From update of attachment 338940 [details] [diff] [review])
> The birthday fields aren't being made read-only on cards from read-only address
> books. This is done in the OnLoadEditCard function - look for if
> (!(directory.operations & directory.opWrite)).

Done, thanks for letting me know about that.
Attachment #338940 - Attachment is obsolete: true
Attachment #338984 - Flags: ui-review?(clarkbw)
Attachment #338984 - Flags: superreview?(bienvenu)
Attachment #338984 - Flags: review+
Attachment #338940 - Flags: ui-review?(clarkbw)
Attachment #338940 - Flags: superreview?(bienvenu)
(Assignee)

Comment 131

9 years ago
Created attachment 338988 [details] [diff] [review]
The real patch v1.4

I accidentally submitted the old patch again and named it 1.4, this is the real patch.
Attachment #338984 - Attachment is obsolete: true
Attachment #338988 - Flags: ui-review?(clarkbw)
Attachment #338988 - Flags: superreview?(bienvenu)
Attachment #338988 - Flags: review+
Attachment #338984 - Flags: ui-review?(clarkbw)
Attachment #338984 - Flags: superreview?(bienvenu)

Comment 132

9 years ago
Comment on attachment 338988 [details] [diff] [review]
The real patch v1.4

landed to get in before string freeze, changeset:   348:428ab4153a1f
Attachment #338988 - Flags: superreview?(bienvenu) → superreview+

Comment 133

9 years ago
I think I'll leave this open for followup issues, but they should probably be put in new bugs, at which point, this should get closed fixed
Comment on attachment 338988 [details] [diff] [review]
The real patch v1.4

this was already signed off in #maildev IRC, just making a note of it now.
Attachment #338988 - Flags: ui-review?(clarkbw) → ui-review+

Updated

9 years ago
Depends on: 455797

Comment 135

9 years ago
FWIW, 
- MoreCols extension way of splitting month and day in 2 fields may be less confusing than x/y regarding locales etc.
- the picker should have the year and month dorpdowns for easy select. Isn't it from calendar? It's there .. Or have separate picker for year.
(In reply to comment #135)
> FWIW, 
> - MoreCols extension way of splitting month and day in 2 fields may be less
> confusing than x/y regarding locales etc.

We could look at extending the label or doing some other form of field. Splitting it would not be appropriate as the date picker then would not work.

> - the picker should have the year and month dorpdowns for easy select. Isn't it
> from calendar? It's there .. Or have separate picker for year.

The date picker is from toolkit not calendar.
Blocks: 456024
Blocks: 456025
Blocks: 456026
I have now filed bugs on the remaining issues that are all linked to this bug.

Bug 456024 - Turn the adapted birthday datepicker into an inherited/extended xbl widget.
Bug 456025 - Implement an anniversary field in the address book.
Bug 456026 - Add day/month order indications to the birthday field on address book contacts.

There is also bug 455797 which is fix some issues with how the birthday field is working.


This bug is therefore FIXED. Other problems/issues should be filed in different bugs if they are not already present.

I'd just like to say thanks to Josh for implementing this feature.
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago9 years ago
Resolution: --- → FIXED

Comment 138

9 years ago
Is there any form of LDAP and/or vCard integration?

I understand that if not, a new bug should be filed. But if the integration already has been taken care of there is no need to do so.

Comment 139

9 years ago
Thanks a lot for these UI improvements! I've shared some thoughts about Lightning integration of birthdays (with ThunderBirthDay) in Bug 456080. Please have at look at them.
Target Milestone: Future → mozilla1.9.1b1
Target Milestone: mozilla1.9.1b1 → Thunderbird 3.0a3
Created attachment 343664 [details]
Birthday field in OS X address book

Its great that a birthday field is available now. Even that we have a synchronization with the OS X address book entries.

But at least I'm a bit concerned about the usability of this control. IMO it's not really handy. Why don't we use a simple text field with dots as separators? The best solution I've found so far is the way how Apple implemented this on OS X. Seeing the birthday date as a whole string and not divided into two different text fields is more like how you would write a birthday in real life. Even when you want to copy the whole date you don't have a chance right now.

Anything we should file as a new bug?
Marco, what is your impression of the usability? Could you have a test with the birthday field?
This bug ("add birthday fields to the addressbok") is fixed.  Please file a new bug for discussions about user experience improvements.
Blocks: 460566
Ok, filed that as bug 460566.

Meanwhile I can verify the integration with following builds:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081017 Shredder/3.0b1pre ID:20081017025444

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b2pre) Gecko/20081016 Shredder/3.0b1pre
Status: RESOLVED → VERIFIED

Updated

9 years ago
Depends on: 465701
Blocks: 271976

Updated

2 years ago
Depends on: 1141620
You need to log in before you can comment on or make changes to this bug.