Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

EZP-28030: Alloy editor add support for more builtin buttons #905

Merged
merged 3 commits into from
Oct 10, 2017
Merged

EZP-28030: Alloy editor add support for more builtin buttons #905

merged 3 commits into from
Oct 10, 2017

Conversation

Plopix
Copy link
Contributor

@Plopix Plopix commented Sep 22, 2017

https://jira.ez.no/browse/EZP-28030

This PR adds 'subscript', 'superscript', 'quote', 'strike', and 'numbered list'
Enjoy
screen shot 2017-09-21 at 6 49 53 pm
screen shot 2017-09-21 at 6 55 54 pm

Todo:

  • make sure kernel can parse/ output all the added syntax

@andrerom andrerom requested review from sunpietro and dew326 October 3, 2017 14:48
*
* @class eZ.AlloyEditor.ButtonList
*/
ButtonList = React.createClass({displayName: "ButtonListIndexed",
Copy link
Member

Choose a reason for hiding this comment

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

I think something is wrong with this generated js. I think this should be ButtonListIndexed = ... instead of ButtonList = ...

@andrerom andrerom self-assigned this Oct 4, 2017
@andrerom
Copy link
Contributor

andrerom commented Oct 4, 2017

Fixed the wrongly generated js.

@Plopix did you test this extensively btw? was the format properly saved to richtext and back to view and edit?

@dew326 looks ok? If so we can potentially send to QA to make sure proper testing is done.
In parallel one of us can try to add some unit tests so this is ready for merge if it passes.

@dew326
Copy link
Member

dew326 commented Oct 5, 2017

@andrerom now it looks ok and I think we can send this to QA.
About testing, I will not be able to focus on this in the current sprint (it's hard to add something extra into the sprint since we are more to scrum).
Let me know if you can handle this or should we try to add this to the upcoming sprint.

@andrerom andrerom changed the title Alloy editor - free button additions EZP-28030: Alloy editor add support for more builtin buttons Oct 5, 2017
@micszo
Copy link
Member

micszo commented Oct 6, 2017

As a smoke test I tried using the new numbered list feature but it looks like there is work on backend needed - XML validation error occurs:
screen shot 2017-10-06 at 10 31 50

@andrerom
Copy link
Contributor

andrerom commented Oct 6, 2017

Thanks @micszo, then we will probably keep this for 1.13 release, it needs a deeper look and work on kernel.

@andrerom
Copy link
Contributor

andrerom commented Oct 8, 2017

@micszo Tested and only found issues with striked: ezsystems/ezpublish-kernel#2103

What browser did you use?

@micszo
Copy link
Member

micszo commented Oct 8, 2017

I used Chrome.

@Plopix
Copy link
Contributor Author

Plopix commented Oct 9, 2017

@andrerom it is in prod for on EE project. I tested the basics, store, edit, view.
I did not test extensively on all the browser though.

Copy link
Member

@mnocon mnocon left a comment

Choose a reason for hiding this comment

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

Looks good: checked editing, viewing and modifying on multiple browsers (Chrome, Safari, Firefox). Tested briefly on Edge, needs broader testing during certification.

@andrerom
Copy link
Contributor

andrerom commented Oct 10, 2017

Thanks @mnocon !

Followups:

  • further testing to try to identify issue reported above
  • add unit tests

@andrerom andrerom merged commit 47de3d3 into ezsystems:master Oct 10, 2017
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants