Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enhancement: elastic skin: allow to force reply all with html format clicking related menu item from button toolbar #7587

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

chilek
Copy link
Contributor

@chilek chilek commented Sep 4, 2020

As in topic - allows to force HTML format for reply from button toolbar. Please merge or review ;-)

@alecpl
Copy link
Member

alecpl commented Sep 4, 2020

FYI, we have a ticket for this already: #5128. I'll review that later.

…clicking related menu item from button toolbar
@alecpl
Copy link
Member

alecpl commented Sep 5, 2020

I don't like it. Here's why

  1. The Reply button menu sounds as a good location only in a first look, but is not.
    • Why only "reply to all in HTML", do we need "reply in HTML" too, what about "reply to list"?
    • The command activity check in this place is too simple (and must be as we have no access to more detailed information about the message here). I'm talking about msg.ctype !== 'text/plain'.
  2. I think the option should be hidden if the user default is already HTML or the message has no HTML part.

So, what other options do we have?

  1. A link/button in the mail preview frame (where Details and Headers links are placed). Note that we might also need similar button for forwarding. In this case we loose possibility to choose between reply and reply to all (and reply to list for that matter).

  2. As suggested in "Reply as HTML" option in "More" menu or elsewhere #5128 we could use Shift key when clicking on Reply/Forward. This however would be hard to discover by users.

  3. So let's place the option in the compose screen. At least in Elastic we have a nice plain text editor toolbar (with one button to switch to HTML mode). We could put "Reply in HTML" (or maybe better "Load HTML body"/"Load original body", so it works the same for forwards) in this toolbar. One note, will we ever need "Load Plain Text body"? If it's needed then the toolbar might not be the best place for that. I don't want to make that an icon-button because it would be confused with the editor mode switching button. So, imho it has to be a text link. It could be also placed in the "Options and attachments" section, I guess.

@chilek
Copy link
Contributor Author

chilek commented Sep 7, 2020

Why only "reply to all in HTML", do we need "reply in HTML" too, what about "reply to list"?

That was the start to check you reaction, so other menu subitems have not been added yet! ;-)

I think the option should be hidden if the user default is already HTML or the message has no HTML part.

Agree.

So let's place the option in the compose screen. At least in Elastic we have a nice plain text editor toolbar (with one button to switch to HTML mode). We could put "Reply in HTML" (or maybe better "Load HTML body"/"Load original body", so it works the same for forwards) in this toolbar. One note, will we ever need "Load Plain Text body"? If it's needed then the toolbar might not be the best place for that. I don't want to make that an icon-button because it would be confused with the editor mode switching button. So, imho it has to be a text link. It could be also placed in the "Options and attachments" section, I guess.

That's the developer style of solution ;-) I think that "mode switch" button should have more sane behavior - I mean when you start replying in plain/text and click this button original html content should be loaded! Adding new button for such purpose would be some kind of hack not convenient for regular user ;-) Did you have a look how other MUA implement this feature?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants