1762126 - (AB-card-UI) [meta] Create new UI for displaying and editing Address Book cards

Status: RESOLVED FIXED

(Thunderbird :: Address Book, enhancement)

Description

[Geoff Lankow (:darktrojan)]

We need a new UI for all of the new capabilities coming to the Address Book.

I'll start this bug with a patch to remove the old UI, then Nicolai will be implementing the new design.

Comment 1

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1762126 - Remove about:addressbook card editing UI for replacement. r=aleca

Comment 2

[u695164]

Attachment: Bug 1762126 - Addressbook contact view with vcard-edit. r=aleca,darktrojan

Comment 3

[u695164]

Attachment: WIP: Bug 1762126 - Save a vCard contact. r=aleca

Comment 4

[u695164]

Attachment: WIP: Bug 1762126 - Integration for vcard-edit in aboutAddressBook view. r=aleca

Comment 5

[u695164]

Attachment: WIP: Bug 1762126 - Edit Contact N-Field L10n, A11Y, styling. r=aleca,henry

Comment 6

[u695164]

Attachment: WIP: Bug 1762126 - Edit Contact N-Field A11Y proposal for input=text fields always in the DOM. r=henry

Comment 7

[u695164]

Attachment: WIP: Bug 1762126 - Edit Contact N-Field A11Y proposal for input=text fields with buttons to be annouced via aria-details. r=henry

Comment 8

[u695164]

Attachment: Bug 1762126 - VCard edit field n tests. r=freaktechnik

Comment 9

[Alessandro Castellani [:aleca]]

I'm not sure what's happening here but when loading the full stack of patches I get this error on startup:

JavaScript error: chrome://messenger/content/msgMail3PaneWindow.js, line 581:
ReferenceError: addressBookTabType is not defined

The app is blank and doesn't finish loading.

Comment 10

[u695164]

(In reply to Alessandro Castellani [:aleca] from comment #9)

I'm not sure what's happening here but when loading the full stack of patches I get this error on startup:

JavaScript error: chrome://messenger/content/msgMail3PaneWindow.js, line 581:
ReferenceError: addressBookTabType is not defined

The app is blank and doesn't finish loading.

Okay sorry, I think it is this deleted line from me:

content/messenger/addressbook/addressBookTab.js (content/addressBookTab.js)
phab link

Seems to be important for macOS but not for linux!
The phab rev D142678 is updated to contain the line.

Comment 11

[u695164]

Attachment: Bug 1762126 - Name field focus after clicking button to reveal suffix, middle name or last name. r=aleca,henry

Comment 12

[u695164]

Attachment: Bug 1762126 - Contact editing sets focus to the first name. r=aleca

Comment 13

[u695164]

Attachment: Bug 1762126 - Add a new contact with vCard. r=darktrojan

Comment 14

[u695164]

Attachment: Bug 1762126 - Delete button for a contact. r=aleca

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/07841155090b
Delete button for a contact. r=aleca

Comment 16

[Geoff Lankow (:darktrojan)]

Wait, what? This is not fixed.

Oops.

Comment 17

[u695164]

Attachment: WIP: Bug 1762126 - Addressbook contact view with vcard-edit. r=aleca,darktrojan

Depends on D143115Differential Revision: https://phabricator.services.mozilla.com/D142678

Comment 18

[u695164]

Attachment: Bug 1762126 - Name field N for the contact edit. r=aleca

Comment 19

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1762126 - Replace about:addressbook card details UI. r=aleca

Comment 20

[u695164]

Attachment: Bug 1762126 - Email field for the contact edit. r=aleca,darktrojan

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/37814bfff8bd
Remove about:addressbook card editing UI for replacement. r=aleca
https://hg.mozilla.org/comm-central/rev/d8e1adac998d
Replace about:addressbook card details UI. r=aleca
https://hg.mozilla.org/comm-central/rev/88331180e0ee
Addressbook contact view with vcard-edit. r=aleca
https://hg.mozilla.org/comm-central/rev/9898569324df
Name field N for the contact edit. r=aleca,darktrojan

Comment 22

[Alessandro Castellani [:aleca]]

Attachment: Bug 1762126 - Various CSS improvements for the Address Book view and edit card. r=darktrojan

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/206296511101
Various CSS improvements for the Address Book view and edit card. r=darktrojan

Comment 24

[u695164]

Attachment: Bug 1762126 - URL field for the contact edit. r=aleca,darktrojan

Comment 25

[u695164]

Attachment: Bug 1762126 - Tel field for the contact edit. r=aleca,darktrojan

Comment 26

[u695164]

Attachment: Bug 1762126 - IMPP field for the contact edit. r=aleca,darktrojan

Comment 27

[u695164]

Attachment: Bug 1762126 - Birthday and anniversary field for the contact edit. r=aleca,darktrojan

Comment 28

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1762126 - Add "role" to the displayed contact properties. r=aleca

Also fixes "organisation" if no department is specified.

Comment 29

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1762126 - Time zone field for the contact edit. r=aleca

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/addac6756321
Add "role" to the displayed contact properties. r=aleca

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f80c874d27c5
Email field for the contact edit. r=aleca,darktrojan
https://hg.mozilla.org/comm-central/rev/44d22e41df10
URL field for the contact edit. r=aleca,darktrojan
https://hg.mozilla.org/comm-central/rev/ceda2825cc76
Tel field for the contact edit. r=aleca,darktrojan
https://hg.mozilla.org/comm-central/rev/d5f5676436e0
Time zone field for the contact edit. r=aleca

Comment 32

[u695164]

Attachment: Bug 1762126 - Address field for the contact edit. r=aleca,darktrojan

Comment 33

[u695164]

Attachment: WIP: Bug 1762126 - Note field for the contact edit. r=aleca,darktrojan

Comment 34

[Alessandro Castellani [:aleca]]

I found a very annoying issue.
It seems that I'm unable to save a new contact:

STR:

• Be in the "All Address Book" list
• Click "New Contact"
• Insert "First Name" and "email"
• Click "Save"

Result:

• A new "contact entry" shows up, but none of the typed info were saved and the contact is completely empty.

Additional issue:

• Be in the "Personal Address Book" list
• Repeat the steps to create a new contact

Result:

• Same empty contact created, but this time the empty list item shows up with a wrong height.

Comment 35

[Thomas D. (:thomas8)]

(In reply to Alessandro Castellani [:aleca] from comment #34)

I found a very annoying issue.
It seems that I'm unable to save a new contact:
Result:

• A new "contact entry" shows up, but none of the typed info were saved and the contact is completely empty.

I had seen that during my testing last week, but then it was only showing Name fields for contact editing in Daily, so I figured it's still WIP. Now more fields are available, and the problem has remained.

Comment 36

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1762126 - Fix the existing Address Book tests for the new UI. r=aleca

This isn't intended to add tests for all of the new UI, just adjust the tests for the old UI to work with the new UI.

Depends on D146790

Comment 37

[Richard Marti (:Paenglab)]

Attachment: Bug 1762126 - Add a tooltip to the default email checkbox. r=aleca

Comment 39

[Alessandro Castellani [:aleca]]

Heads up for a needed change, which I think it's a simple fix, but the "Default" radio button shouldn't be visible if only one email address field is currently used.

Comment 40

[Henry Wilkes (they/them) [:henry-x]]

I looked at the email addresses input table, and the design seems slightly broken.

Currently it is

<!-- These labels seem to be duplicates, and very far away structurally from where they are used. -->
<label id="vcard-email-table-header-type-label" hidden="">Email address</label>
<label id="vcard-email-table-header-email-label" hidden="">Email address</label>
<!-- This label shouldn't have both text content and an aria-label. Moreover, the aria-label is not an appropriate label for a checkbox. -->
<label id="vcard-email-table-header-primary" hidden="" aria-label="Choose your primary email address">Default</label>
...
<table>
  <thead>
    <tr>
      <th scope="col">Type</th>
      <th scope="col">Email address</th>
      <!-- Same issue with the aria-label as above. Also, this is hidden when creating a new contact. -->
      <th scope="col" aria-label="Choose your primary email address" hidden="">Default</th>
    </tr>
  </thead>
  <tbody>
    <!-- I'm not sure why a role of "row" was set. -->
    <tr is="vcard-email" slot="v-email" role="row">
      <td>
        <select class="vcard-type-selection" aria-labelledby="vcard-email-table-header-type-label">
        </select>
      </td>
      <td>
        <input type="email" aria-labelledby="vcard-email-table-header-email-label" />
      </td>
      <td>
        <input type="checkbox" aria-labelledby="vcard-email-table-header-primary" />
      </td>
    </tr>
  </tbody>
</table>

Whereas a simpler design would be (following https://webaim.org/techniques/forms/advanced#multiple)

<!-- No extra labels. -->
<table>
  <thead>
    <tr>
      <th scope="col" id="vcard-email-table-header-type-label">Type</th>
      <th scope="col" id="vcard-email-table-header-email-label">Email address</th>
      <!-- "Default" or "Default email" is a fine label for the checkbox. -->
      <th scope="col" id="vcard-email-table-header-primary">Default</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <td>
        <!-- Labelled by the heading. -->
        <select class="vcard-type-selection" aria-labelledby="vcard-email-table-header-type-label">
        </select>
      </td>
      <td>
        <input type="email" aria-labelledby="vcard-email-table-header-email-label" />
      </td>
      <td>
        <input type="checkbox" aria-labelledby="vcard-email-table-header-primary" />
      </td>
    </tr>
  </tbody>
</table>

Please let me know if there is some specific problem that motivated the current design and maybe I can give a solution.

Comment 41

[Henry Wilkes (they/them) [:henry-x]]

Also, this bug introduced the class "screen-readers-only" (with an "s" after "reader") and defines it exactly the same in two places https://searchfox.org/comm-central/search?q=screen-readers-only&path=css&case=false&regexp=false

But we also have the class "screen-reader-only" (with no "s" after "reader") with the same rules https://searchfox.org/comm-central/rev/cfb1ede00a8093b57b1643743f84edddb9001a43/mail/themes/shared/mail/messageHeader.css#554

We should just have one class that we share.

Comment 42

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d61db4470cfb
Fix the existing Address Book tests for the new UI. r=aleca

Comment 43

[Henry Wilkes (they/them) [:henry-x]]

(In reply to Henry Wilkes [:henry] from comment #40)

I looked at the email addresses input table, and the design seems slightly broken.
...

Also mentioned in bug 1717632 comment 5, so this can be addressed there instead.

Comment 44

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1762126 - Add "prefer display name" checkbox. r=aleca

Comment 45

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1762126 - Notes UI for contact editor. r=aleca

Comment 46

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8c16942886b2
IMPP field for the contact edit. r=aleca,darktrojan
https://hg.mozilla.org/comm-central/rev/6d88f82c0e10
Birthday and anniversary field for the contact edit. r=aleca,darktrojan
https://hg.mozilla.org/comm-central/rev/1d8b8d9fe427
Address field for the contact edit. r=aleca,darktrojan
https://hg.mozilla.org/comm-central/rev/a7738c6bdba2
Add "prefer display name" checkbox. r=aleca

Comment 47

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1762126 - Organisational properties UI for contact editor. r=aleca

I've kept the title, role, department and organisation fields grouped together. This needed a bit of special handling but it works.

Comment 48

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1d8cca6141b2
Notes UI for contact editor. r=aleca

Comment 49

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9a6f0f764a6c
Organisational properties UI for contact editor. r=aleca

Comment 50

[u695164]

Attachment: Bug 1762126 - VCard edit strings for further fields. r=darktrojan,aleca

Comment 51

[Andreas]

I saw some issues with the URL field.

First: I can't enter thunderbird.net or www.thunderbird.net, it has to be https://thunderbird.net

Second: Save a website for a contact, go to another contact, go back to the first contact and open the edit form. The URL is now changed to https://thunderbird.net with an additional backslash \
I could only reproduce this with a CardDAV addressbook (in my case Google), not with a local one.

Comment 52

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1762126 - Remove aria-label for addressbook primary email label. r=aleca

Comment 53

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/22c44a417a21
Remove aria-label for addressbook primary email label. r=aleca

Comment 54

[Rob Lemley [:rjl]]

Comment on attachment 9278788 [details]
Bug 1762126 - Remove aria-label for addressbook primary email label. r=aleca

[Triage Comment]
Approved for 102.0b1.

Comment 55

[Rob Lemley [:rjl]]

Thunderbird 102.0beta1:
https://hg.mozilla.org/releases/comm-beta/rev/de511f8862d5

Comment 56

[Thomas D. (:thomas8)]

Wayne mentioned on Matrix that this bug is at least meta-ish. I agree.
We've landed a thousand things here, and we're still creating more satellite bugs which we wouldn't want on the top-level (tb-new-addrbook) bug.
Let's give this bug some more visibility and make it more obvious that lots of (patches and) AB bugs are linked up (only) on this one.

Comment 57

[Magnus Melin [:mkmelin]]

(In reply to Andreas from comment #51)

I saw some issues with the URL field.

Please file a new bug. It's expected that only a real url will be allowed though, not just "www.example.com".

Comment 58

[Richard Marti (:Paenglab)]

Attachment: Bug 1762126 - Hide the border in contact edit fieldset and align the textfields. r=aleca

Comment 59

[u695164]

(In reply to Henry Wilkes [:henry] from comment #40)

I looked at the email addresses input table, and the design seems slightly broken.

Currently it is

<!-- These labels seem to be duplicates, and very far away structurally from where they are used. -->
<label id="vcard-email-table-header-type-label" hidden="">Email address</label>
<label id="vcard-email-table-header-email-label" hidden="">Email address</label>
<!-- This label shouldn't have both text content and an aria-label. Moreover, the aria-label is not an appropriate label for a checkbox. -->
<label id="vcard-email-table-header-primary" hidden="" aria-label="Choose your primary email address">Default</label>
...
<table>
  <thead>
    <tr>
      <th scope="col">Type</th>
      <th scope="col">Email address</th>
      <!-- Same issue with the aria-label as above. Also, this is hidden when creating a new contact. -->
      <th scope="col" aria-label="Choose your primary email address" hidden="">Default</th>
    </tr>
  </thead>
  <tbody>
    <!-- I'm not sure why a role of "row" was set. -->
    <tr is="vcard-email" slot="v-email" role="row">
      <td>
        <select class="vcard-type-selection" aria-labelledby="vcard-email-table-header-type-label">
        </select>
      </td>
      <td>
        <input type="email" aria-labelledby="vcard-email-table-header-email-label" />
      </td>
      <td>
        <input type="checkbox" aria-labelledby="vcard-email-table-header-primary" />
      </td>
    </tr>
  </tbody>
</table>

Whereas a simpler design would be (following https://webaim.org/techniques/forms/advanced#multiple)

<!-- No extra labels. -->
<table>
  <thead>
    <tr>
      <th scope="col" id="vcard-email-table-header-type-label">Type</th>
      <th scope="col" id="vcard-email-table-header-email-label">Email address</th>
      <!-- "Default" or "Default email" is a fine label for the checkbox. -->
      <th scope="col" id="vcard-email-table-header-primary">Default</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <td>
        <!-- Labelled by the heading. -->
        <select class="vcard-type-selection" aria-labelledby="vcard-email-table-header-type-label">
        </select>
      </td>
      <td>
        <input type="email" aria-labelledby="vcard-email-table-header-email-label" />
      </td>
      <td>
        <input type="checkbox" aria-labelledby="vcard-email-table-header-primary" />
      </td>
    </tr>
  </tbody>
</table>

Please let me know if there is some specific problem that motivated the current design and maybe I can give a solution.

Yeah it's currently broken.
We agreed on your point.

My issue occured because the slot element is a problem for a table and stuff like

<select>
    <slot></slot>
</select

It's not flattening the HTML tree. Currently it's XML compliant but not HTML. The role="row" is added to for the accessibility tree with a broken table layout.

For more information see:
https://github.com/WICG/webcomponents/issues/59#issuecomment-202330706
https://github.com/WICG/webcomponents/issues/404#issuecomment-202290604

I'd like to get the slot element out. I will file a bug for it with possible solutions and add you if its fine for you.

Edit:

The labels on the top are there because of an issue with the shadow dom borders.
Without the slot element there's no need for the shadow dom and therefore no need for these additional labels.
They should be removed.

Comment 60

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/83ea13ba136d
Hide the border in contact edit fieldset and align the textfields. r=aleca

Comment 61

[Richard Marti (:Paenglab)]

Comment on attachment 9278948 [details]
Bug 1762126 - Hide the border in contact edit fieldset and align the textfields. r=aleca

[Approval Request Comment]
User impact if declined: On Windows unneeded borders and on all platforms not well aligned textboxes
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): low

Comment 62

[Henry Wilkes (they/them) [:henry-x]]

(In reply to Nicolai Kasper from comment #59)

I'd like to get the slot element out. I will file a bug for it with possible solutions and add you if its fine for you.

ok, sounds good. The slots also made it a lot harder to understand the area because it splits the DOM into a different order. This also made it harder to access in the Browser Toolbox.

Please also note, from bug 1717632 comment 5:

The "Websites" and "Phone Numbers" similarly have multiple entries, but are structured differently to the "Email Addresses" section. They should be consistent and use a table as well.

I think they also use slots, so maybe you can address this in the same bug as well.

Comment 63

[Geoff Lankow (:darktrojan)]

Calling this done.

Comment 64

[Wayne Mery (:wsmwk)]

Comment on attachment 9278948 [details]
Bug 1762126 - Hide the border in contact edit fieldset and align the textfields. r=aleca

[Triage Comment]
Approved for beta

Comment 65

[Rob Lemley [:rjl]]

Thunderbird 102.0beta6:
https://hg.mozilla.org/releases/comm-beta/rev/09d200e063d7