(Persistent) "Reply to sender" button on header should not be dropdown

RESOLVED FIXED in Thunderbird 8.0

Status

defect
--
trivial
RESOLVED FIXED
10 years ago
7 years ago

People

(Reporter: bugzilla2007, Assigned: squib)

Tracking

Thunderbird 8.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

10 years ago
On new header pane toolbar, with "Always show Reply to Sender" enabled, we show a separate "Reply to sender" button next to the intelligent "Reply all/list/whatever" dropdown.

Actual
- (Persistent) Reply to sender button is a dropdown

Expected
- (Persistent) Reply to sender button should not be dropdown, because it never shows more than 1 item, namely "reply" itself
- it should just be a plain button

Bryan (who implemented persistent reply button, thanks!), you think you could fix this? Having a functionless second dropdown is actually quite odd and confusing.
Reporter

Comment 1

10 years ago
This is a minor (partial/incomplete) followup from Bug 511924.
Reporter

Updated

10 years ago
Blocks: 511924
Assignee

Comment 2

8 years ago
Posted patch Fix this (obsolete) — Splinter Review
This has been annoying me for ages, and I finally got annoyed enough to fix it. It's a bit more complicated than "make the reply button a regular button", since news shows a "reply" button that has "reply all" as a sub-option, but it's still pretty simple.

I hesitate to write tests for this right now though, since I think I'll be stepping on herb's toes over in bug 519956 if I do so. Nevertheless, tests should be pretty straightforward.
Assignee: nobody → squibblyflabbetydoo
Status: NEW → ASSIGNED
Attachment #541987 - Flags: review?(bwinton)
Comment on attachment 541987 [details] [diff] [review]
Fix this

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

I mostly like this patch.  r=me with the nits below fixed.

::: mail/base/content/mailWindowOverlay.js
@@ +996,5 @@
> +  let smartReplyButton    = document.getElementById("hdrSmartReplyButton");
> +  let replyOnlyButton     = document.getElementById("hdrReplyOnlyButton");
> +  let replyButton         = document.getElementById("hdrReplyButton");
> +  let replyAllButton      = document.getElementById("hdrReplyAllButton");
> +  let replyListButton     = document.getElementById("hdrReplyListButton");

Please don't line these up.  It makes it harder to add stuff later.  (There's also another example or two further down that I'ld like you to change…)

::: mail/base/content/msgHdrViewOverlay.xul
@@ -212,0 +212,5 @@
> > +                <toolbarbutton id="hdrReplyOnlyButton" label="&hdrReplyButton.label;"
> > +                               tooltiptext="&hdrReplyButton.tooltip;"
> > +                               oncommand="MsgReplyMessage(event);RestoreFocusAfterHdrButton();"
> > +                               observes="button_reply"
> > +                               class="toolbarbutton-1 msgHeaderView-button hdrReplyToSenderButton"/>

Should this toolbarbutton have the hdrReplyToSenderButton class, or should we add a new class for it, or should it use the hdrReplyButton class?
Attachment #541987 - Flags: ui-review+
Attachment #541987 - Flags: review?(bwinton)
Attachment #541987 - Flags: review+
Assignee

Comment 4

8 years ago
(In reply to comment #3)
> Should this toolbarbutton have the hdrReplyToSenderButton class, or should
> we add a new class for it, or should it use the hdrReplyButton class?

Hmm. In general, it should always look like the reply to sender button, but maybe a theme would want to make one of them different. I'm not sure that the hdrReplyButton class is right, since that's a dual-button, and this is just a regular button. I guess the most flexible solution is to give it a new class.

One other thing I noticed that's kind of confusing: when you have the "reply to sender" button and the "smart reply" button on the toolbar, and you customize it while looking at a message sent only to you (i.e. the "smart reply" button is showing "reply only"), you see two seemingly identical reply buttons, so you don't know which one is the smart reply and which one is the reply to sender.

I'm not totally sure how to fix that, but here are a couple of possibilities:

 1) give the "smart reply" toolbaritem a label so at least you know what it is
    in the Customize dialog
 2) when customizing and the smart reply button is on the toolbar, change its
    label to be "smart reply" (maybe by way of a dummy button)
 3) when customizing and the smart reply button is on the toolbar, set it to the
    "reply all" mode to disambiguate it

Any preferences?
Assignee

Comment 5

8 years ago
I went with (1) and (2) from comment 4 with this patch. I also cleaned up the UpdateReplyButtons function a bit more, so the logic there should be clearer.

Forewarning: I'm not sure if labels on toolbaritems show up in the Customize dialog on Mac, so the smart reply button might not have a label there. If that's still an issue, it's probably somewhere in toolkit, since I know it works in Linux (and I'm pretty sure it works in Windows too).
Attachment #541987 - Attachment is obsolete: true
Attachment #545060 - Flags: review?(bwinton)
Comment on attachment 545060 [details] [diff] [review]
Make customization work better

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

Okay, I've checked this over, and it seems okay, so I guess that means r=me!

Thanks,
Blake.
Attachment #545060 - Flags: review?(bwinton) → review+
Assignee

Comment 7

8 years ago
Checked in: http://hg.mozilla.org/comm-central/rev/b9ce47b3670b
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 8.0
Assignee

Updated

7 years ago
Duplicate of this bug: 545930
You need to log in before you can comment on or make changes to this bug.