Rich editor component

[tnajdek]

Used for editing notes and such.

[dstillman]

I assume this is a placeholder, but we'll of course want to use TinyMCE here for compatibility with the client (though we can consider other text editors for future versions).

[tnajdek]

While this is an early/placeholder implementation, I was hoping we could avoid using TinyMCE as it won't work that nice with react.

Ultimately, what is produced by a rich text editor is a piece of html. We can create (using draft-js framework/utilities) a rich text editor that produces (and understands) identical set of tags/classes/styles as does the TinyMCE. Wouldn't that be a more elegant solution?

[dstillman]

It depends on the extent to which we can customize Draft's logic, but the problem arises when the HTML uploaded to the API is cleaned differently from what the API does. The API response then includes a different serialization, which for safety the API consumer should update itself with, but it can be difficult to maintain cursor position through a refresh of the HTML, particularly if the user is actively typing. It's much better for a save to not require a local refresh, so that changes are only going in one direction. (Draft actually describes a similar problem, and they advise against having the delayed save affect the editor state.) We've had issues like this in Zotero for years, resulting from using TinyMCE on the client and HTMLPurifier on the server, and we had all sorts of HTML normalization logic in the 4.0 conflict resolution code. I've finally switched us to using TinyMCE's cleaning logic on the server to avoid these problems.

We also want the same note experience everywhere, so I don't think it makes sense to use something different on the web library before we decide what other editor we'll use across the board — which will be a pretty critical decision, and I don't think we'd limit our choices to editors that are designed specifically for React (though Draft may very well be a solid choice). Until we do pick another editor, it doesn't seem like running TinyMCE in React would be a problem — have you looked at the third-party module they link to?

[tnajdek]

Ok, I see your point, it won't be as elegant integration but it makes sense to stick with TinyMCE.

[tnajdek]

Rich editor is working but is very limited at the moment. We're not rendering any of the default styling for TinyMCE which would stand out and would not fit the rest of the design. Instead I've opted for rendering buttons in our app and communicate with TinyMCE via their execCommand and queryCommandState. This gives us the best flexibility, however it also means we need to prepare/collect icons for all desired functionality.

For comparison here's what functionality client offers:

[flachware]

That sounds good to me. We already have some icons we need in Tropy and we can reuse newly created icons in the client too.

[flachware]

@tnajdek some buttons do not work yet (quote, alignment, lists, etc.), why is that?

[tnajdek]

hey @flachware great job styling the editor! Few minor questions before I can finish this and close the issue:

• text color dropdowns have been configured to only open when pressing the caret icon, wouldn't it be better if the entire icon worked as a dropdown trigger?
• with recent changes (probably tinymce update), rich editor content is no longer visible on touch devices. Setting height on .rich-editor would fix it but I'm wondering if you had something else in mind?
• on touch devices, modal doesn't seem to be wide enough to fit the entire toolbar. I assume we're ignoring this because of changes suggested in https://github.com/zotero/web-library/issues/219#issuecomment-509269872 ?

[flachware]

@tnajdek thanks!

• Not sure if I understand your question correctly, but I stuck to the client here, pressing the left side of the split button applies the currently selected color whereas the right side opens the dropdown to select a color. Does that answer your question?

• I broke that some days ago, don’t worry about it for now. We will stretch the editor using flexbox, like we do on desktops (no need to set height 100% anymore at all). I just didn’t work on the touch version yet.

• Yes I’d ignore the modal for now because of #219. I think I will be able to squeeze in all the tools on touch with some tweaks, but if not, a '...' dropdown with the remaining tools should not be too hard to implement in this case.

[tnajdek]

pressing the left side of the split button applies the currently selected color

ahh, ok it makes sense then, thanks for explaining!

We will stretch the editor using flexbox, like we do on desktops (no need to set height 100% anymore at all)

Not sure if this is relevant but tinymce will add height: 100% to .tox-tinymce which comes from js config for tinymce. As far as I can tell I cannot prevent it from setting it, but I can set it to "auto" instead if that helps (currently it breaks the editor though)

[flachware]

You can leave the height as it is, the height property gets overwritten by flexbox anyway.

[flachware]

@tnajdek currently the active class on toolbar buttons is set like that: when you click an inactive button the active class is set on mouse up and when you click the active button it is removed on mousedown.

Could we set/unset the active class in both cases on mouse up?

[tnajdek]

@flachware I've implemented the remaining functionality in the rich editor. Color picker styling might need some attention, especially the 'clear' button which probably needs an icon.

Any issues let me know or if you're happy, let's close the issue.

[tnajdek]

Could we set/unset the active class in both cases on mouse up?

This was to do with blur triggering on mouse down on the button, causing button active state to change. Currently when editor looses focus, all buttons will also loose "active" state. Come to think about it, perhaps we don't need to do it?

[flachware]

@tnajdek I’m having a look at the editor.

Could we close the color picker when you click inside the editor (plus on ESC)?

Come to think about it, perhaps we don't need to do it?

Don’t need to do what? (The current behavior of the active state looks good to me.)

[tnajdek]

Don’t need to do what? (The current behavior of the active state looks good to me.)

When you focus on something else than editor (or editor's toolbar), all buttons become inactive. Otherwise it seems weird to have some buttons active based on what was the state of the editor when you blurred it. However looking at the client, it does exactly that (i.e. preserves the active buttons when focused on something else) so I'm thinking maybe we should do the same

[flachware]

Yes I think it makes sense to keep the active state. E.g. you select some text, hit bold and click outside of the editor. The focus is gone now, but the text selection is still there, thus it makes sense to show the active states of the currently selected text, even when the focus is somewhere else.

[flachware]

@tnajdek when a paragraph is indented, the outdent button gets the active class, which I guess doesn’t make sense as indent/outdent aren’t binary toggles (it it doesn’t happen in the client either).

Also, it looks like indent/outdent can be used in the client to sink and lift the level of list items, could you have a look at that?

[flachware]

The touch editor only gets initialized properly for me when I switch an open note from mouse to touch, otherwise I end up with an empty textarea (without iframe etc.) – is this work in progress? Not sure if this is related at all, but I get an Uncaught SyntaxError: Unexpected token < from theme.js when loading the web-library.

[flachware]

@tnajdek I’ve added some media queries for lg touch devices that handle the toolbar overflow. By now there should be no overflow anymore on those devices, instead certain tool groups get hidden and a '...' dropdown-toggle appears that would provide options for the hidden tools.

Could you add the following options to the dropdown menu (I will show/hide them with CSS according to the viewport width)?

Toolbar 1: sup sub – removeformat – blockquote link

Toolbar 2: bullet list numbered list indent outdent

[flachware]

I think it doesn’t make sense to show the toolbars on phones, I guess the best way would be to not insert them at all in this case? Or should I simply hide them with CSS?

[tnajdek]

I think it doesn’t make sense to show the toolbars on phones, I guess the best way would be to not insert them at all in this case? Or should I simply hide them with CSS?

You mean the rich editor toolbars? Why not?

[tnajdek]

@flachware "link" and "search and replace" will not work on some devices. This apparently done "by design" in tinymce...

Since we're not using their "mobile" theme, we could modify tinymce to always assume desktop, but that probably means we will need to restyle modals they produce. It also adds maintenance as we will need to do this anytime there is an update.

Thus I think we could just hide this functionality on touch/small devices?

[flachware]

You mean the rich editor toolbars? Why not?

Yes the rich editor toolbars. Below is an iPhone SE screenshot of how the layout would look like with toolbars. There isn’t really much left of the note you want to edit. The buttons of the first toolbar are already clipped a bit and most tools would be in the overflow dropdown menu – which is not scrollable so far and would not be fully visible (same is true for the format dropdown). Color pickers wouldn’t fully fit in the screen. And we would rather have to show tools with dropdowns (as we don’t have a dropdown submenu) in the toolbar than frequently used ones to make this work.

So that’s why I suggest to reduce the functionality to editing without format options on phones (that’s basically what Basecamp does).

[flachware]

@tnajdek re 'link' and 'search and replace', I read that TinyMCE 5 comes with both a desktop and a mobile theme built in, I guess we are only using the desktop theme on purpose?

Having a look at this mobile demo https://codepen.io/k644606347/pen/WyRxoW it looks like it wouldn’t be of much help on small devices for the tricky issues (large dropdown menus, dropdown submenus). And for some reason the color, link, and search features are missing here.

In terms of styling, the current editor modals are alien elements in the interface, I think I will have to style them anyway, so I could also tweak the editor’s desktop theme for mobiles. I agree that we should keep maintenance at a minimum, but we already do some restyling for other components, like the react-select.

From my point of view the preferable approach would be to use our own modals for the editor dialogs – but I assume that would cause a lot of effort on your side or would not be possible at all?

[flachware]

@tnajdek as concluded, please do not initialize the rich editor toolbars on phones.

[flachware]

@tnajdek could we have a CSS file that gets loaded in the iframe after TinyMCE’s content.min.css? We will need some adjustments at least, it would be helpful if this CSS file derives from an SCSS file, so we can use theme variables.

[tnajdek]

@flachware I've added tinymce-content.scss which is loaded in the iframe

[flachware]

@tnajdek I’ve just realized that media queries treat the iframe like a viewport, which means that existing media queries about viewport widths do not work correctly here. I also can’t know if the web library is in touch mode from inside the iframe.

Could you set a .touch class on the body element (html element would be even better) whenever isTouchOrSmall applies?

[tnajdek]

@flachware I've added .touch on body element in the iframe if device is small or touch. Bear in mind that if user switches viewport while editing a note, this is value does not change (navigating away and back fixes this). This is slightly tricky to fix because the entire editor needs to be reinitialized to reflect the change which may lead to annoying experience.

[flachware]

@tnajdek alright, thanks.

[flachware]

@tnajdek the .touch class on body seems to update fine with me, I don’t have to navigate away and back?

[flachware]

@tnajdek I’d have two TinyMCE requests:

TinyMCE sets an inline style with the line-height on each paragraph – and a color on the h1. Is there a way to make it not do this?

When you create a link in the editor the link text is editable, unless you press the Cmd key (macOS), then clicking the link opens the link target. Could you set a .meta-key class on the body element inside the iframe when the meta key is pressed so I can switch the text cursor to pointer and show the hover text decoration?

[flachware]

One more request: could you set an .active class on button.dropdown-item in the format-block dropdown menu so I can highlight the current format?

[dstillman]

Are we still on TinyMCE 3? If possible, we should upgrade to TinyMCE 4 and enable the link plugin with the link_context_toolbar option, which shows a toolbar when left-clicking a link, which makes it much easier to interact with and open links. (In TinyMCE 3 we used our own custom version to do the same thing, but there's a built-in version in TinyMCE 4.)

[tnajdek]

@dstillman We're using TinyMCE 5, I've enabled link_context_toolbar, it seems to work though the menu it opens is considerably larger:

[dstillman]

OK, yeah, I saw that style in the current demo on the TinyMCE site. That's fine.

[tnajdek]

TinyMCE sets an inline style with the line-height on each paragraph – and a color on the h1. Is there a way to make it not do this?

@flachware This might be possible via content formatting override. However this will break compatibility with TinyMCE editors elsewhere, most importantly desktop client. Text edited by TinyMCE elsewhere will still apply these styles, while TinyMCE in web-library won't. Even if we would also override content formatting in the client that would not apply to notes that already exist. Thus I don't think we can do this.

[tnajdek]

When you create a link in the editor the link text is editable, unless you press the Cmd key (macOS), then clicking the link opens the link target. Could you set a .meta-key class on the body element inside the iframe when the meta key is pressed so I can switch the text cursor to pointer and show the hover text decoration?

Done

One more request: could you set an .active class on button.dropdown-item in the format-block dropdown menu so I can highlight the current format?

Done

[flachware]

@tnajdek re content formatting, I can override the set inline styles in any case, but where do those inline styles come from? Can they be configured?

[flachware]

@tnajdek where are we regarding the Insert/Edit Link and Find and Replace modals (and now also the link context toolbar)? Can we use our own modals or should I simply override the TinyMCE styles?

[tnajdek]

@tnajdek re content formatting, I can override the set inline styles in any case, but where do those inline styles come from? Can they be configured?

My point is that I'm not sure we want to adjust these. Wouldn't we want notes to look identical in web-library and in the client?

@tnajdek where are we regarding the Insert/Edit Link and Find and Replace modals (and now also the link context toolbar)? Can we use our own modals or should I simply override the TinyMCE styles?

I think we should just override TinyMCE styles.

[flachware]

@tnajdek yes I think we can basically stick to the values that get set by inline styles. Ok, I’ll override the TinyMCE modal and toolbar styles then.

[flachware]

Open tasks:

• [x] Style context menu on touch devices
• [ ] Let groups in link context toolbar wrap in second line
• [ ] Style link context toolbar on touch devices
• [ ] Style URL suggestions
• [ ] Missing borders and x icon in color picker on touch devices