Closed Bug 1466055 Opened 6 years ago Closed 5 years ago

Style the EditContactPanel buttons like the FX panel footer buttons

Categories

(Thunderbird :: Theme, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 65.0

People

(Reporter: Paenglab, Assigned: Paenglab)

Details

Attachments

(1 file, 3 obsolete files)

I played a bit with styling the EditContactPanel buttons like the ones the panels of FX use. And this is what I got.

Funnily a bit later FX also begun to change the buttons of the bookmark panel (bug 1462166) which is about to land.
Attached patch EditContactPanel.patch (obsolete) — Splinter Review
Jörg, what do you think about this? The colours are the same as FX uses.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8982449 - Flags: review?(jorgk)
Comment on attachment 8982449 [details] [diff] [review]
EditContactPanel.patch

(In reply to Richard Marti (:Paenglab) from comment #1)
> Jörg, what do you think about this?
Another "spot the difference" review ;-)
So you click on the star next to the e-mail address and the "Edit Contact" panel opens. Before you had "simple" buttons, now you have those mobile-phone-style buttons. Frankly, I don't like it much, especially since the delete button is still a "simple" button. Unless you find a more integrated approach, I don't think it's ready for review yet.
Attachment #8982449 - Flags: review?(jorgk) → feedback?(acelists)
Comment on attachment 8982449 [details] [diff] [review]
EditContactPanel.patch

Review of attachment 8982449 [details] [diff] [review]:
-----------------------------------------------------------------

The difference is quite large for me, when you told which dialog we are talking about:)
I wonder why any css changes are needed here. This "doorhanger" style with buttons should be in Toolkit and we should have been using it for a long time now.
(But it seems the trend is to have no Toolkit, just open-code all the style hacks everywhere in the browser. Otherwise XUL would have a replacement.)
For me also the "delete" button got flat now.
What did you mean with "simple" button? That is isn't connected with the edges of the panel as the other buttons are?
Attached patch EditContactPanel.patch (obsolete) — Splinter Review
Moved the "Delete" button to the bottom and removed the "Cancel" button. FX's bookmark panel has also no "Cancel" button. Clicking outside the panel also cancels the changes, the changes are only saved with clicking the "Done" button.
Attachment #8982449 - Attachment is obsolete: true
Attachment #8982449 - Flags: feedback?(acelists)
Attachment #8984661 - Flags: review?(jorgk)
Attachment #8984661 - Flags: feedback?(acelists)
Comment on attachment 8984661 [details] [diff] [review]
EditContactPanel.patch

I don't think I like this. At least the buttons should be equally big. I can just hear Aceman saying: More FX ugliness ;-(
Attachment #8984661 - Flags: review?(jorgk)
Comment on attachment 8984661 [details] [diff] [review]
EditContactPanel.patch

Review of attachment 8984661 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry, this needs some unbitrotting in css files.

Yes, of course this is "FX ugliness", but today we already have a doorhanger when installing addons. So this at least brings the 'edit contact' panel to the same style.

The buttons will probably never be the same width, it may also depend on the length of the labels in different languages.
Hey, that's a bug right there, if the 'edit card' text is longer, the 'button' does not accomodate it and the label overwrites the next button. This needs to be solved universally for proper translation.
Attachment #8984661 - Flags: feedback?(acelists) → feedback+
(In reply to :aceman from comment #6)
> Comment on attachment 8984661 [details] [diff] [review]
> EditContactPanel.patch
> 
> Review of attachment 8984661 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Sorry, this needs some unbitrotting in css files.

Done

> Yes, of course this is "FX ugliness", but today we already have a doorhanger
> when installing addons. So this at least brings the 'edit contact' panel to
> the same style.

It doesn't look so bad.

> The buttons will probably never be the same width, it may also depend on the
> length of the labels in different languages.
> Hey, that's a bug right there, if the 'edit card' text is longer, the
> 'button' does not accomodate it and the label overwrites the next button.
> This needs to be solved universally for proper translation.

Fixed this. It should now work with every text length.
Attachment #8984661 - Attachment is obsolete: true
Attachment #9027142 - Flags: review?(acelists)
Comment on attachment 9027142 [details] [diff] [review]
1466055-EditContactPanel.patch v 2

I can live with this.
Attachment #9027142 - Flags: feedback+
Comment on attachment 9027142 [details] [diff] [review]
1466055-EditContactPanel.patch v 2

Review of attachment 9027142 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, this works. But could there be at least some visual separation between the buttons, like a thin vertical line?
Attachment #9027142 - Flags: feedback+
(In reply to :aceman from comment #9)
> Comment on attachment 9027142 [details] [diff] [review]
> 1466055-EditContactPanel.patch v 2
> 
> Review of attachment 9027142 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks, this works. But could there be at least some visual separation
> between the buttons, like a thin vertical line?

There was a rule for the border but it didn't apply. Fixed.
Attachment #9027142 - Attachment is obsolete: true
Attachment #9027142 - Flags: review?(acelists)
Attachment #9027310 - Flags: review?(acelists)
Comment on attachment 9027310 [details] [diff] [review]
1466055-EditContactPanel.patch v3

Review of attachment 9027310 [details] [diff] [review]:
-----------------------------------------------------------------

Nice, thanks :)
Attachment #9027310 - Flags: review?(acelists) → review+
Comment on attachment 9027310 [details] [diff] [review]
1466055-EditContactPanel.patch v3

Review of attachment 9027310 [details] [diff] [review]:
-----------------------------------------------------------------

Nice, thanks :)
Keywords: checkin-needed
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/e43bbffbc0c3
Style the EditContactPanel buttons like the FX panel footer buttons. r=aceman
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 65.0
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: