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

EZP-29328: As an editor I want AE to support formatted text #709

Merged
merged 6 commits into from
Nov 14, 2018

Conversation

dew326
Copy link
Member

@dew326 dew326 commented Nov 7, 2018

Question Answer
Tickets https://jira.ez.no/browse/EZP-29328
Bug fix? no
New feature? yes
BC breaks? no
Tests pass? yes
Doc needed? yes
License GPL-2.0

Screenshot:
formatted

This is a new feature but the target branch is 1.3 as this is needed for the migration.

Checklist:

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

Copy link
Contributor

@andrerom andrerom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Did you figure out what caused the toolbar position to go all over the place @dew326 ?

@dew326
Copy link
Member Author

dew326 commented Nov 8, 2018

@andrerom yes, it's because the empty pre tag gets some empty text nodes. Hopefully it's fixed in this line: https://github.com/ezsystems/ezplatform-admin-ui/pull/709/files#diff-71c5e1a664b1342cfae2e62de8b241bbR19

Copy link
Contributor

@sunpietro sunpietro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one minor change.

Copy link

@katarzynazawada katarzynazawada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Formatted style is not always passible to choose
  1. Click on Rich Text Editor
  2. Change style to Formatted.
  3. Type same text.
  4. Change style to Heading 1.

There is no way to change the style to formatted – there is no Formatted block in the list. When changing style to Paragraph Formatted block appears.

  • 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.

picture1

  • RichText Editor is not styled properly when changing styles

A) Safari

  1. Click on Rich Text Editor
  2. Change style to Formatted.
  3. Type same text.
  4. Add new block (Paragraph).
  5. Change Paragraph style to Formatted.
  6. Change the style back to Paragraph.

screenshot 2018-11-08 at 15 55 44

B) Chrome

Steps like on Safari and then:

  1. Click outside the editor.

  2. Go back to Rich Text Editor.

screenshot 2018-11-08 at 15 52 33

After removing block and clicking in Editor:

screenshot 2018-11-08 at 16 05 19

Safari:

  • Toolbar skips:
  1. Click on Rich Text Editor
  2. Change style to Formatted.

picture2

  • Pre use scroll is not added. (on Edge also)

screenshot 2018-11-08 at 13 22 13

@katarzynazawada
Copy link

After fixes:

  • Pre use scroll is not added on Firefox

  • Cursor is outside the block frame on Safari

  1. Click on Rich Text Editor
  2. Change style to Formatted.
  3. Type same text.
  4. Add new block (Paragraph).
  5. Change Paragraph style to Formatted.
  6. Change the style back to Paragraph.

screenshot 2018-11-13 at 08 47 02

@dew326
Copy link
Member Author

dew326 commented Nov 13, 2018

@katarzynazawada
As discussed in private first issue is because "lorem ipsum" copy paragraph with styles and we cannot do anything with that. If the text is unstyled or typed into the field the scroll is added.

The second issue is not related to our code base. I can reproduce the issue on AlloyEditor demo. I created an issue for AE devs: liferay/alloy-editor#902

@lserwatka lserwatka merged commit 4e855ee into ezsystems:1.3 Nov 14, 2018
@lserwatka
Copy link
Member

@dew326 you can merge it up

@dew326 dew326 deleted the ae-formatted-support branch November 14, 2018 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

6 participants