Use HTML input instead of XUL textbox in abCard.inc.xul
Categories
(Thunderbird :: General, task)
Tracking
(Not tracked)
People
(Reporter: aleca, Assigned: aleca)
References
Details
Attachments
(1 file, 7 obsolete files)
48.42 KB,
patch
|
aleca
:
review+
|
Details | Diff | Splinter Review |
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 1•3 years ago
|
||
New contact dialog ready for review.
The Photo tab doesn't really work, but it's not related to the input conversion.
I reported it on bug 1581253.
Comment 2•3 years ago
|
||
Comment on attachment 9092815 [details] [diff] [review] 1563118-textbox-html-input.patch Review of attachment 9092815 [details] [diff] [review]: ----------------------------------------------------------------- Work | Department is hosed. ::: mail/components/addrbook/content/abCard.inc.xul @@ +147,5 @@ > <spacer flex="1"/> > <label control="WorkPhone" value="&WorkPhone.label;" > accesskey="&WorkPhone.accesskey;" /> > + <html:input id="WorkPhone" > + type="text" for the phone numbers, type="tel" (for correctness) @@ +220,5 @@ > accesskey="&HomeAddress2.accesskey;"/> > + <hbox class="AddressCardEditWidth input-container"> > + <html:input id="HomeAddress2" > + type="text" > + class="input-inline"/> for this and others were it would all fit well on one line, I'd keep it all on one line @@ +316,5 @@ > <label control="Department" value="&Department.label;" > accesskey="&Department.accesskey;"/> > + <html:input id="Department" > + type="text" > + class="input-inline"/> this one is not showing @@ +589,5 @@ > <radio id="WebPhotoType" value="web" label="&PhotoURL.label;" > accesskey="&PhotoURL.accesskey;"/> > + <hbox class="indent input-container items-stretch"> > + <html:input id="PhotoURI" > + type="text" type="url" ::: mail/themes/osx/mail/addrbook/cardDialog.css @@ +30,5 @@ > .placeholderOption { > color: GrayText; > } > > +html|input.YearWidth { maybe for these it's preferable to remove the default xul namespace instead...
Assignee | ||
Comment 3•3 years ago
|
||
The department field shows up for me, how does it look for you?
Regarding the Address double line, I'd suggest to leave it as it's pretty common to have 2 address lines for extra info, like apartment #, buzzer, etc.
I noticed that the label for the second line doesn't actually have any text.
Should we add it for accessibility?
Comment 4•3 years ago
|
||
I notice there are other things off too.
Re "keep" it on one line, i was referring to the actual code. (That you now put each attribute on its own line.)
Assignee | ||
Comment 5•3 years ago
|
||
Yeah, I noticed now the Stat/Province line being busted.
I'm reworking the patch, and applying the aria-labelledby
attribute as well.
Assignee | ||
Comment 6•3 years ago
|
||
Comment 7•3 years ago
|
||
Comment on attachment 9094275 [details] [diff] [review] 1563118-textbox-html-input.patch Review of attachment 9094275 [details] [diff] [review]: ----------------------------------------------------------------- Nope. At least Department and Zip hosed, like in the screenshot
Assignee | ||
Comment 8•3 years ago
|
||
This is so weird.
What about this?
Comment 9•3 years ago
|
||
Comment on attachment 9094278 [details] [diff] [review] 1563118-textbox-html-input.patch Review of attachment 9094278 [details] [diff] [review]: ----------------------------------------------------------------- I'll attach a screenshot
Comment 10•3 years ago
|
||
Assignee | ||
Comment 11•3 years ago
|
||
I think I figured it out.
For some reason your fields have a min-width
that doesn't allow them to properly "flex" into the limits of their container.
It doesn't happen on mine, it might be a HiDPI thing.
Working on a fix.
Assignee | ||
Comment 12•3 years ago
|
||
This should do it.
Comment 13•3 years ago
|
||
Comment on attachment 9094668 [details] [diff] [review] 1563118-textbox-html-input.patch Review of attachment 9094668 [details] [diff] [review]: ----------------------------------------------------------------- Yeah, looks good now. ::: mail/components/addrbook/content/abCard.inc.xul @@ +354,3 @@ > <label control="Age" value="&Age.label;"/> > + <html:input id="Age" > + type="number" please add min="0" too
Assignee | ||
Comment 14•3 years ago
|
||
Patch updated and try-run pushed: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=b50186e4b9b764edfeb4e41079bb1980897a579c
This will remove 48 textbox
elements, yay!
Comment 15•3 years ago
|
||
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/06921c904e77
Use HTML input instead of XUL textbox in abCard.inc.xul. r=mkmelin
Updated•3 years ago
|
Description
•