EZP-29328: As a editor I want OE to support formatted text #676

andrerom · 2018-10-08T10:20:43Z

Question	Answer
Tickets	EZP-29328
Bug fix?	yes and no
New feature?	yes and no
BC breaks?	no
Tests pass?	yes (there are none for js code)
Doc needed?	yes
License	GPL-2.0

Besides being new feature in eZ Platform Admin UI, this allows migration of plain literal tags to eZ Platform as supported since 2.3.0 in kernel and migration scripts. This is why this is suggested for 1.3.1, to avoid customers reporting it as bug once they migrate.

Checklist:

Coding standards ($ composer fix-cs)
Ready for Code Review

andrerom · 2018-10-08T14:47:24Z

Updated* and rebased, so ready for review.

* Removed webpack changes not relevant, needed to keep changes in all toolbars where option for formatted is shown, they change because of the change in /base.js here which they extend.

dew326 · 2018-10-09T06:10:46Z

src/bundle/Resources/public/js/alloyeditor/src/toolbars/config/ez-formatted.js

+    constructor(config) {
+        super(config);
+
+        this.name = 'paragraph';


Probably not a paragraph

andrerom · 2018-10-09T08:03:02Z

src/bundle/Resources/public/js/alloyeditor/src/toolbars/config/ez-formatted.js

+            'ezmoveup',
+            'ezmovedown',
+            this.getStyles(config.customStyles),
+            // NOTE: alignment currently not supported on <pre> so skipping buttons for that


@vidarl Actually did not check if this is supported or not on pre/programlisting, besides noticing there where no handling of this in the xsl. However if we did not have it in legacy either there is probably no need to add it on formatted.

We do not have align in legacy ( editor disables align buttons ) on literal.
Docbook does not support it on ProgramListing either, but we are free to add support for ezxhtml:align attribute if we choose to.

However, since it was not a need for it in legacy, it is likely no needed now either.

andrerom · 2018-10-09T08:04:33Z

@dew326 updated 👍

Note that I have not added this to the "Add block" toolbar, I think we should, but then we probably need a new icon for it and I'm unsure how you guys do that now.

lserwatka · 2018-10-09T08:17:53Z

OK, let's move it to QA, we could make 2.3.1 on Friday as I have some other fixes in the pipeline.

katarzynazawada

During the tests I noticed several bugs on different browsers. For other blocks they don’t occur.

Firefox:

Cursor appears in the block frame only when you start to write.

Safari:
After switching to Formatted toolbar skips.

Chrome:
For more text, scroll appears in edit mode. If clicking Add button in situation when cursor is outside the current view, page and toolbar skips.

andrerom · 2018-10-12T08:49:06Z

@katarzynazawada which of these issues are unique to the change here? As in, safari issue and chrome add button issue sounds like its unrelated => generic that might also happen with edge cases on other blocks, or?

The Firefox and the chrome scroll bar issues are probably due to <pre> styling, I/someone might need to check it against Alloy's css and see if issue comes from there or if it comes from our own css.

andrerom · 2018-10-12T08:59:56Z

@katarzynazawada If you have those browsers still running, could you cross check against https://alloyeditor.com/ ? (front page, scroll down to "In live demo, click/touch here")

katarzynazawada · 2018-10-12T10:33:12Z

@andrerom I’ve checked these issues again both on master and your branch. From my observations, all of them only appear for Formatted block. What is more, on Chrome, there is one more case:  

Click on Rich Text Editor 
Change style to Formatted. 
Click outside the editor. 
Go back to Rich Text Editor.

  Result:

Besides being new feature in eZ Platform, this allows migration of plain literal tags to eZ Platform, as recently added in migration script.

andrerom · 2018-10-21T13:45:21Z

Pushed CSS that tries to solve some of the issues.

@lserwatka btw some of those seems fixed in Alloy 1.5.x: liferay/alloy-editor#867, liferay/alloy-editor#868, liferay/alloy-editor#888

@dew326 do you have suggestion on what might cause the remaining toolbar issue on empty tag? I see Alloy adds a br inside paragraphs which is probably to avoid this, anyway we can emulate that in CSS too?

andrerom · 2018-10-28T18:19:35Z

~~Pushed ezplatform-admin-ui-assets 3.2.0-rc1 to test with newer Alloy version, feel free to remove it.~~

On closer look the toolbar issue here is not caused by Alloy, as it's our own toolbar. Code responsible for remaining set of toolbar position bugs is class EzConfigBase.

andrerom · 2018-10-29T20:49:58Z

@katarzynazawada Added fix for:

toolbar position when scrolling* & when empty**
height of formatted box when empty***
made pre use scroll in all browsers (added comment inline in css for how you can enable wrapped style instead, but non wrapped is the normal behavior for pre, so I stayed with that)
added <> when empty as placeholder.

You should not need to update Alloy for this, in the end the bugs where caused by our code, so you don't need new asset package to test this.

For devs:
* Needed to take .scrollLeft into account.
** Caused by block not being considered empty by code.
*** Added pre:empty::after where some placeholder content is set when empty.

katarzynazawada · 2018-10-30T12:51:42Z

@andrerom After fixes I noticed some issues:

Safari

Toolbar skips:

A)

Click on Rich Text Editor
Change style to Formatted.
Click outside Rich Text Editor but within a short distance.
Click on Rich Text Editor again.

See movie: http://recordit.co/tsiwhvKKza

B)

Click on Rich Text Editor
Change style to Formatted.
Change style to Paragraph (or another one)

Chrome:

Placeholder sometimes disappears 

Click on Rich Text Editor
Change style to Formatted.
Change style to Paragraph (or another one)
Change style to Formatted again.

After publishing ,in Content item view, there is a blank space in field instead of This field is empty text

Firefox:

Pre use scroll is not added.

If clicking Add button in situation when cursor is outside the current view, page and toolbar skips. - this issue still occurs for scroll.

andrerom · 2018-10-30T13:41:07Z

@SylvainGuittard If you want this feature, please get someone else to take over at some point. The level of deep issues in Alloy / browsers here is beyond what I have time to reverse engineer any time soon.

andrerom · 2018-10-31T11:06:49Z

UPDATE: Placeholder issue was fixed in branch here in 45feb2f (by removing placeholder, it's not important). However the toolbar issue is something else, maybe deeper in the editor, so if we want to cover all edge cases on first try here, it's better if a frontend guy can dig into it. /cc @lserwatka

This comment has been minimized.

Sign in to view

andrerom force-pushed the oe_add_formatted_text branch from 4af7c26 to f8d0b96 Compare October 8, 2018 14:45

andrerom requested review from tischsoic and dew326 October 8, 2018 14:46

dew326 requested changes Oct 9, 2018

View reviewed changes

andrerom commented Oct 9, 2018

View reviewed changes

dew326 approved these changes Oct 9, 2018

View reviewed changes

lserwatka added the Ready for QA label Oct 9, 2018

tischsoic approved these changes Oct 9, 2018

View reviewed changes

katarzynazawada requested changes Oct 11, 2018

View reviewed changes

katarzynazawada self-assigned this Oct 11, 2018

andrerom added 2 commits October 18, 2018 17:49

[OE] Add formated text option as now supported by kernel as of 7.3

0eb1241

Besides being new feature in eZ Platform, this allows migration of plain literal tags to eZ Platform, as recently added in migration script.

Fix Review note & add comment on alignment

16fa7ed

andrerom changed the base branch from master to 1.3 October 21, 2018 13:37

andrerom force-pushed the oe_add_formatted_text branch from 1f7f530 to 3b1d640 Compare October 21, 2018 13:38

Add styling to pre tag to try to avoid Alloy Toolbar & styling issues

dd3e1e5

andrerom force-pushed the oe_add_formatted_text branch from 3b1d640 to dd3e1e5 Compare October 21, 2018 13:57

This comment has been minimized.

Sign in to view

Fix scroll position of toolbar, change back to overflow by default

0e9947a

This comment has been minimized.

Sign in to view

Fix special cases in Safari, Firefox and Chrome

ea09680

andrerom closed this Oct 30, 2018

ViniTou added a commit that referenced this pull request May 12, 2023

Added missing translations (#676)

62975bc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EZP-29328: As a editor I want OE to support formatted text #676

EZP-29328: As a editor I want OE to support formatted text #676

andrerom commented Oct 8, 2018 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

andrerom commented Oct 8, 2018 •

edited

Loading

dew326 Oct 9, 2018

andrerom Oct 9, 2018

andrerom Oct 9, 2018 •

edited

Loading

vidarl Oct 9, 2018

andrerom commented Oct 9, 2018 •

edited

Loading

lserwatka commented Oct 9, 2018

katarzynazawada left a comment

andrerom commented Oct 12, 2018 •

edited

Loading

andrerom commented Oct 12, 2018

katarzynazawada commented Oct 12, 2018

andrerom commented Oct 21, 2018 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

andrerom commented Oct 28, 2018 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

andrerom commented Oct 29, 2018 •

edited

Loading

katarzynazawada commented Oct 30, 2018

andrerom commented Oct 30, 2018

andrerom commented Oct 31, 2018 •

edited

Loading

EZP-29328: As a editor I want OE to support formatted text #676

EZP-29328: As a editor I want OE to support formatted text #676

Conversation

andrerom commented Oct 8, 2018 • edited Loading

Checklist:

This comment has been minimized.

This comment has been minimized.

andrerom commented Oct 8, 2018 • edited Loading

dew326 Oct 9, 2018

Choose a reason for hiding this comment

andrerom Oct 9, 2018

Choose a reason for hiding this comment

andrerom Oct 9, 2018 • edited Loading

Choose a reason for hiding this comment

vidarl Oct 9, 2018

Choose a reason for hiding this comment

andrerom commented Oct 9, 2018 • edited Loading

lserwatka commented Oct 9, 2018

katarzynazawada left a comment

Choose a reason for hiding this comment

andrerom commented Oct 12, 2018 • edited Loading

andrerom commented Oct 12, 2018

katarzynazawada commented Oct 12, 2018

andrerom commented Oct 21, 2018 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

andrerom commented Oct 28, 2018 • edited Loading

This comment has been minimized.

This comment has been minimized.

andrerom commented Oct 29, 2018 • edited Loading

katarzynazawada commented Oct 30, 2018

andrerom commented Oct 30, 2018

andrerom commented Oct 31, 2018 • edited Loading

andrerom commented Oct 8, 2018 •

edited

Loading

andrerom commented Oct 8, 2018 •

edited

Loading

andrerom Oct 9, 2018 •

edited

Loading

andrerom commented Oct 9, 2018 •

edited

Loading

andrerom commented Oct 12, 2018 •

edited

Loading

andrerom commented Oct 21, 2018 •

edited

Loading

andrerom commented Oct 28, 2018 •

edited

Loading

andrerom commented Oct 29, 2018 •

edited

Loading

andrerom commented Oct 31, 2018 •

edited

Loading