WYSIWYG editor: find acceptable editor

tomholub commented 5 years ago

(but do not actually add any formatting buttons yet)

can consider https://github.com/quilljs/quill

The editor should have an easy to use, cross-browser way to extract plain text if the user just wants to send plain text.

depends on #1522 prerequisite to #668

The solution should be as lightweight as possible, with the following parameters:

small / clean codebase
minimalistic look (should not look like a ms word product from 2005)
can do bold, underline, italic, strike-through, text color, text size, bullet points
has a remove formatting feature
affordable. Either open source under a permissive license (BSD, MIT, Apache2) or with a paid solution under $1,000 USD (lifetime license) or under 300 USD (yearly license).
well supported - this means either active open source maintainer, or a well supported paid product

Optional features (nice to have):

has a source code quoting feature, like slack or keybase chat. For example with backticks, or triple-backticks like GitHub. Not a hard requirement, but I've always wanted to have this in email :smiley:

To do

please evaluate a few paid and open source solutions, try out their demos, and let me know which 2 or 3 of them we should consider, and your thoughts about them. Please link to the online demos below for me to play with them.

michael-volynets commented 5 years ago

I was using 'quill' before, so I'm a little experienced with this tool but I was using it within Angular 7. Perhaps there is a better alternative for the native JS/TS. I will look for it and return here with some other tools.

michael-volynets commented 5 years ago

Here are some editors I have found:

Free:

Quill (https://quilljs.com/playground/) - 240KB JS + CSS
Summernote (https://summernote.org/) - 80 KB JS + CSS
Trumbowyg (https://alex-d.github.io/Trumbowyg/) - Ultra lightweight 20KB only
Prosemirror (http://prosemirror.net/) - ???

I found some paid plugins and some of them are really expensive and give us functionality that we do not need, others are cheaper but they give the same functionality as free.

So, what do I think about those? I think firstly better to look at Trumbowyg and Summernote because they are lightweight editors. If they don't fit we need to take a look at Quill and Prosemirror

tomholub commented 5 years ago

My notes:

Trumbowyg - a little unpolished but working. Adding inline images works, but cannot make them smaller after adding (like in Gmail).
- Summernote - seems more polished & supports image resizing. I like the air mode: https://summernote.org/examples/#air-mode . It seems to depend on Bootstrap, but there is also a light mode without bootstrap. But no example of the light mode.
- Quill - simple & nice but cannot resize images :(
- Prosemirror - seemed broken when I added a screenshot (big image)

I think I like Summernote the best. Thanks for finding it! Can you please make a simple html page with Summernote in it? The light version, and in air mode. You can load the libraries from CDN and just paste the page html here, for me to test. Thanks!

michael-volynets commented 5 years ago

Sure, Here you go:

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8">
    <title>bootstrap4</title>
    <script src="https://code.jquery.com/jquery-3.2.1.slim.min.js"></script>
    <link href="https://cdnjs.cloudflare.com/ajax/libs/summernote/0.8.12/summernote-lite.css" rel="stylesheet">
    <script src="https://cdnjs.cloudflare.com/ajax/libs/summernote/0.8.12/summernote-lite.js"></script>
  </head>
  <body>
    <div id="summernote"></div>
    <script>
      $('#summernote').summernote({
        placeholder: 'Hello stand alone ui',
        tabsize: 2,
        height: 500,
        airMode: true,
        popover: {
          air: [
            ['font', ['bold', 'italic', 'underline', 'clear']],
            ['para', ['ol', 'ul', 'paragraph']],
          ]
        },      
        disableResizeEditor: true
      });
    </script>
  </body>
</html>

tomholub commented 5 years ago

Looks great! A few issues: 1) cannot resize image after inserting anymore 2) when pasting an image, it will double it:

Can you please see if these are configuration issues, or has something to do with being the light version?

michael-volynets commented 5 years ago

Hmm, I don't know why but I can resize the image. And yes, it doubles the image when pasting an image. I will try to find some more info about this issue

michael-volynets commented 5 years ago

But actually, there is another UI issue that I've noticed. You can't insert an image without highlighting some text. To insert a picture you must have to highlight some text, then upload your image and finally, the text will be replaced with the image. I don't think it's a good way to add a picture to the message.

tomholub commented 5 years ago

On my end, on Firefox, I don't need to highlight anything to add an image. Just click in the text filed and ctrl + v.

I suspect the lite version is buggier than the full one, probably less used and less tested.

tomholub commented 5 years ago

Maybe try to also test the full version with bootstrap. This plugin is nice overall, so maybe keeping bootstrap for it is worth it, if the full version is well tested.

tomholub commented 5 years ago

Also, please update config:

remove formatting icon to be at the end
add hyperlink icon
font size icon
font style choice (only 3 options: Serif, Sans serif, Monospace).
font color (you can skip background color)

michael-volynets commented 5 years ago

Okay, Shouldn't we add add picture in the config?

tomholub commented 5 years ago

I highly dislike the add picture modals that pop up, they are usually ugly and confusing.

But now that you mention it.. some people may not know how to add pictures otherwise.. so yes. But see if you can make it work without a modal dialog. When I click add picture, it should just open the same choose file system window that opens when I click add attachment, with no UI of our own.

Inline pictures should be directly base64 encoded in the html.

michael-volynets commented 5 years ago

Yes, I agree with you about modals. I will try to make it work the way you said

michael-volynets commented 5 years ago

Summernote supports custom user plugins. I've written some here (flowcryptaddpicture):

<!DOCTYPE html>
<html lang="en">

<head>
  <meta charset="UTF-8">
  <title>Summernote</title>
  <link href="http://netdna.bootstrapcdn.com/bootstrap/3.3.5/css/bootstrap.css" rel="stylesheet">
  <script src="http://cdnjs.cloudflare.com/ajax/libs/jquery/3.2.1/jquery.js"></script>
  <script src="http://netdna.bootstrapcdn.com/bootstrap/3.3.5/js/bootstrap.js"></script>
  <link href="http://cdnjs.cloudflare.com/ajax/libs/summernote/0.8.12/summernote.css" rel="stylesheet">
  <script src="http://cdnjs.cloudflare.com/ajax/libs/summernote/0.8.12/summernote.js"></script>
</head>

<body>
  <div id="summernote">
    <p style="font-family: 'Serif'">Hello Summernote</p>
    <input type="file" style="display: none"; id="file_input" />
  </div>
  <script>
(function(factory) {
    /* global define */
    if (typeof define === 'function' && define.amd) {
        // AMD. Register as an anonymous module.
        define(['jquery'], factory);
    } else if (typeof module === 'object' && module.exports) {
        // Node/CommonJS
        module.exports = factory(require('jquery'));
    } else {
        // Browser globals
        factory(window.jQuery);
    }
}(function($) {

    $.extend($.summernote.plugins, {
        'modal': function(context) {
            var self = this;

            var ui = $.summernote.ui;

            context.memo('button.flowcryptaddpicture', function() {
                var button = ui.button({
                    contents: '<i class="note-icon-picture"/>',
                    tooltip: 'Add picture',
                    click: function() {
                      debugger;
                      var input = document.getElementById('file_input');
                      input.onchange = event => { 
                        var data = event.target.files || event.target.value;

                        context.invoke('editor.restoreRange');
                        if (typeof data === 'string') { // image url
                          // If onImageLinkInsert set,
                          if (self.options.callbacks.onImageLinkInsert) {
                            context.triggerEvent('image.link.insert', data);
                          } else {
                            context.invoke('editor.insertImage', data);
                          }
                        } else { // array of files
                          context.invoke('editor.insertImagesOrCallback', data);
                        }
                      }

                      input.click();
                    }
                });

                // create jQuery object from button instance.
                var $hello = button.render();
                return $hello;
            });
        }
    });
}));

    $(document).ready(function () {
      $('#summernote').summernote({
        airMode: true,
        defaultFontName: 'Serif',
        fontNames: ['Serif', 'Sans Serif', 'Monospace'],
        popover: {
          air: [
            ['font', ['fontname', 'bold', 'italic', 'underline', 'fontsize', 'forecolor']],
            ['insert', ['link', 'flowcryptaddpicture']],
            ['para', ['ol', 'ul']],
          ]
        }

      });
    });
  </script>
</body>

</html>

Here I added a custom plugin to add a picture without their dialog.

Also, I updated the config according to the one you wrote but there is one problem with font names, I'm not able to remove Arial and Helvetica fonts, it's hardcoded in their codebase. Can you check if it's suitable?

tomholub commented 5 years ago

It's good. I like the img pasting.

But when I paste img, still cannot choose size. It seems to auto-choose size based on the initial window width, which

tomholub commented 5 years ago

which is not the worst. Tested on Firefox.

tomholub commented 5 years ago

Part of #1621

tomholub commented 5 years ago

One thing I didn't like is that it shows no don't by default. Should be default Arial

tomholub commented 5 years ago

No font

tomholub commented 5 years ago

At this point, the next steps will be:

[x] is it possible to make it auto-select a font, like Arial? And when I select some text I did not choose a font for yet, it would just say Arial.
[ ] Actually, I would like to rename Arial to just Sans Serif, and make that the default. Is that possible?

michael-volynets commented 5 years ago

I found another issue in Summernote. To add a picture you can either use your keyboard (CTRL + V) or use the button in the popover. Adding picture via the keyboard is simple and works well but some people may not know about that. So they will use adding a picture through Summernote popover: but to open the popover you need to select some text and then the popover will be opened. I don't think this is the best UI for pasting images because you need to select an area, click on the picture button and then the selected area will be replaced with the image you've selected.

I think would be better to remove the picture button from the popover and add it somewhere in our composer, for example here: but before I need to investigate that if it's possible to add a button (the button will be outside summernote container) that will invoke some method in summernote.

What do you think?

michael-volynets commented 5 years ago

Also, I updated the config according to the one you wrote but there is one problem with font names, I'm not able to remove Arial and Helvetica fonts, it's hardcoded in their codebase. Can you check if it's suitable

Also, I downloaded sources of Summernote and fixed an issue with Arial And Helvetica fonts. (These were required fonts and we couldn't remove them) I know that's bad practice to change something inside the library because we can't update the library anymore. I did it because I couldn't find other ways to fix the issue with fonts. Here is what I've changed in Summernote source code to fix the issue:

-      // Add 'default' fonts into the fontnames array if not exist
-       $.each(styleInfo['font-family'].split(','), (idx, fontname) => {
-        fontname = fontname.trim().replace(/['"]+/g, '');
-        if (this.isFontDeservedToAdd(fontname)) {
-          if (this.options.fontNames.indexOf(fontname) === -1) {
-            this.options.fontNames.push(fontname);
+      if (this.options.addDefaultFonts) {
+        // Add 'default' fonts into the fontnames array if not exist
+        $.each(styleInfo['font-family'].split(','), (idx, fontname) => {
+          fontname = fontname.trim().replace(/['"]+/g, '');
+          if (this.isFontDeservedToAdd(fontname)) {
+            if (this.options.fontNames.indexOf(fontname) === -1) {
+              this.options.fontNames.push(fontname);
+            }
          }
-        }
-      });
+       });
+      }

If it's okay for you, we can use the changed version of Summernote.

michael-volynets commented 5 years ago

I found another issue in Summernote. To add a picture you can either use your keyboard (CTRL + V) or use the button in the popover. Adding picture via the keyboard is simple and works well but some people may not know about that. So they will use adding a picture through Summernote popover: but to open the popover you need to select some text and then the popover will be opened. I don't think this is the best UI for pasting images because you need to select an area, click on the picture button and then the selected area will be replaced with the image you've selected.

I think would be better to remove the picture button from the popover and add it somewhere in our composer, for example here: but before I need to investigate that if it's possible to add a button (the button will be outside summernote container) that will invoke some method in summernote.

What do you think?

I've added the button outside the Summernote and it works. I will send you it in next example

michael-volynets commented 5 years ago

Here is the latest version of Summernote config. Can I start implementing it in our composer?

<!DOCTYPE html>
<html lang="en">

<head>
  <meta charset="UTF-8">
  <title>Summernote</title>
  <link href="http://netdna.bootstrapcdn.com/bootstrap/3.3.5/css/bootstrap.css" rel="stylesheet">
  <script src="http://cdnjs.cloudflare.com/ajax/libs/jquery/3.2.1/jquery.js"></script>
  <script src="http://netdna.bootstrapcdn.com/bootstrap/3.3.5/js/bootstrap.js"></script>
  <link href="http://cdnjs.cloudflare.com/ajax/libs/summernote/0.8.12/summernote.css" rel="stylesheet">
  <script src="http://cdnjs.cloudflare.com/ajax/libs/summernote/0.8.12/summernote.js"></script>
</head>

<body>
  <button id="add_picture">Add Picture</button>
  <div class="container" style="max-width: 100%">
      <div id="summernote">
          <p style="font-family: 'Arial'">Hello Summernote</p>
          <input type="file" style="display: none"; id="file_input" />
        </div>
  </div>
  <script>
(function(factory) {
    /* global define */
    if (typeof define === 'function' && define.amd) {
        // AMD. Register as an anonymous module.
        define(['jquery'], factory);
    } else if (typeof module === 'object' && module.exports) {
        // Node/CommonJS
        module.exports = factory(require('jquery'));
    } else {
        // Browser globals
        factory(window.jQuery);
    }
}(function($) {

    $.extend($.summernote.plugins, {
        'modal': function(context) {
            var self = this;

            var ui = $.summernote.ui;
            $('#add_picture').click(function() {
                      var input = document.getElementById('file_input');
                      input.onchange = event => { 
                        var data = event.target.files || event.target.value;

                        context.invoke('editor.restoreRange');
                        if (typeof data === 'string') { // image url
                          // If onImageLinkInsert set,
                          if (self.options.callbacks.onImageLinkInsert) {
                            context.triggerEvent('image.link.insert', data);
                          } else {
                            context.invoke('editor.insertImage', data);
                          }
                        } else { // array of files
                          context.invoke('editor.insertImagesOrCallback', data);
                        }
                      }

                      input.click();
                    })
        }
    });
}));

    $(document).ready(function () {
      $('#summernote').summernote({
        airMode: true,
        focus: true,
        defaultFontName: 'Serif',
        fontNames: ['Serif', 'Sans Serif', 'Monospace', 'Helvetica', 'Arial'],
        popover: {
          air: [
            ['font', ['fontname', 'bold', 'italic', 'underline', 'fontsize', 'forecolor']],
            ['insert', ['link']],
            ['para', ['ol', 'ul']],
          ],
          image: [
            ['float', ['floatLeft', 'floatRight', 'floatNone']],
            ['remove', ['removeMedia']]
          ]
        }
      });
    });
  </script>
</body>

</html>

tomholub commented 5 years ago

Yes I wanted to have the image button outside the whole time anyway.

You could make an issue @ summernote repository, say that you need the font choices configurable, and ask them if they are open to a PR. Then you can implement it as a configurable option and get it merged upstream.

I'll take a look at this config.

michael-volynets commented 5 years ago

I found an issue when I added bootstrap to Flowcrypt. We have some classes that called the same way as bootstrap called them (for example: container).

So if we use bootstrap we will need to change our classes that have the same name as bootstrap has.

tomholub commented 5 years ago

Some pages already use bootstrap. Look into libs/

michael-volynets commented 5 years ago

Okay, now I see.

that was my mistake, There is .container class in compose.htm but it doesn't have any styles. So I just removed it. Thanks

tomholub commented 5 years ago

On both chrome and Firefox, I do not have to have any text selected to add an image with the button.

tomholub commented 5 years ago

Also the fonts still seem silly. Should say (in this order):

Sans Serif
Serif
Monospace

With Sans Serif as the default.

Currently on Firefox:

Currently on Chrome:

Let's not start integrating it in compose just yet. But if you need to do some preparation (like remove the .container class), no prob, we can merge that separately first.

tomholub commented 5 years ago

I worry that bootstrap will screw around with the compose screen layout in general. Therefore we could make another PR, only adding bootstrap and fixing all the problems that arise with it.

Actually, does your plugin need the bootstrap? Or is it still possible to continue evaluating this plugin without bootstrap (lite version)? That would be best, if possible.

michael-volynets commented 5 years ago

As I remember, we had some issues with the lite version but I can check it again.

Let's not start integrating it in compose just yet. But if you need to do some preparation (like remove the .container class), no prob, we can merge that separately first.

What do you mean? Do you want me firstly to refactor compose.htm (remove bootstrap classes) in another PR, and then return to this again? Or you want me to switch on another issue?

tomholub commented 5 years ago

For now, let's only focus on preparing summernote to look ready for integration, but do not start integrating it yet.

We cannot yet integrate it because FlowCrypt does not yet support sending pgp/mime, it's on my list. Once it's merged, we can start working on integrating summernote into compose.

Please check again if bootstrap is necessary. I think the main issue was with images, which you solved with a plugin.

tomholub commented 5 years ago

michael-volynets commented 5 years ago

See, I found Summernote Lite older version (0.8.11, the current version is 0.8.12). It doesn't depend on bootstrap and works quite well. It doesn't double images (like in version 0.8.2). It also doesn't look very well, so we need to change some styles there. I think we could use this version of Summernote but I need to test it little more, maybe I will find some other issues.

tomholub commented 5 years ago

That means 0.8.12 works worse than 0.8.11?

Just tell the developers, it's probably a regression. They'll fix it.

michael-volynets commented 5 years ago

yes, exactly. They already have this issue created on their Github repository.

tomholub commented 5 years ago

Let's expect to work with 0.8.12. If they can't fix it, we'll fix it. Don't spend time fixing up styles on an older version.

That means we can drop bootstrap, which is great news.

Please look into the fonts https://github.com/FlowCrypt/flowcrypt-browser/issues/1630#issuecomment-512360169

michael-volynets commented 5 years ago

Issues:

When you paste a large image, you cannot resize it without scroll. The resize button is located in the bottom-right corner and when you paste a large image we can't see the bottom-right corner because it can't fit in the whole window.

To fix that we can add bottom scroll so Flowcrypt user will be able to access bottom-right corner or we can resize the image before we pasted it into our composer. (Second way of fixing I like more but should be more investigated how we can do it)

In the lite version, we can see these checkmarks when we choose font-name and font-size:

This is UI issue and not a critical one but I think would be better to remove them.

Plain Text mode This editore doesn't support Plain Text mode but I checked how Gmail has done that. They just sanitize text before pasting it in the composer. I mean if I have some HTML text in my clipboard and then I'm trying to paste it into Gmail Composer, Gmail gets plain text of what I've pasted. (We even can see how HTML renders and then it replaced with plaintext alternative)

michael-volynets commented 5 years ago

Let's expect to work with 0.8.12. If they can't fix it, we'll fix it. Don't spend time fixing up styles on an older version.

if we will fix that so we will need to fix the doubling images issue. (Which is only in 0.8.12 version)

michael-volynets commented 5 years ago

Also, I found another issue that's only in 0.8.12: When you're resizing the image the gray box (look at the screenshot)

The original size of the image was like the gray square is then I increased the size of the image but the gray square still has the same size. (In 0.8.11 it's increasing together with image)

michael-volynets commented 5 years ago

Seems like the newer version has more bugs than the older one :smile:

tomholub commented 5 years ago

I'm starting to wonder if Summernote is the right choice. I'm afraid they'll keep breaking it on us. The main reason to go with Summernote was resizing images built in. But it's all too buggy.

Please try the following: https://quilljs.com/ https://github.com/kensnyder/quill-image-resize-module

And paste a html file for me to play with, like you did with summernote.

I bet it will be a lot better after all.

tomholub commented 5 years ago

Plus there is also air mode! The Bubble theme: https://quilljs.com/docs/themes/#bubble

michael-volynets commented 5 years ago

Honestly, that looks much better than Summernote.

I've got an issue with Font-Name & Font-Size now. When I finish I will send you on email the archive with the result.

tomholub commented 5 years ago

You can make new branch, with a new folder experiment/ in it, and push it there. Then give me a link to the branch here. That will be easier.

michael-volynets commented 5 years ago

Here you go: https://github.com/FlowCrypt/flowcrypt-browser/tree/experimental/issue-1630

michael-volynets commented 5 years ago

Look into Quill Editor Folder in the project root

tomholub commented 5 years ago

I think it's awesome. Honestly, I think it's better than what Google does.

tomholub commented 5 years ago

Plus I can resize the image to any size I want. No small/medium/original. Anything I want. Woohoo! :laughing:

FlowCrypt / flowcrypt-browser

WYSIWYG editor: find acceptable editor #1630

To do