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

Introduced the editor quick actions toolbar #130

Merged
merged 2 commits into from
Nov 30, 2021
Merged

Introduced the editor quick actions toolbar #130

merged 2 commits into from
Nov 30, 2021

Conversation

oleq
Copy link
Member

@oleq oleq commented Nov 19, 2021

Feature: Introduced editor quick actions toolbar (see #38) to log editor data, toggle editor read-only, and destroy editor. Closes #121. Closes #104.


Notes

Under the hood, 

  • I refactored the Button component to be more versatile because the number of buttons increased and some need isOn/isEnabled states. 
  • Also, I changed the way icons are handled in the project (loaded from SVG instead of hardcoded base64), unified their size, and made them compatible with mentioned buttons.
  • Toolbar separators are spans with a class now for reusability because they appear in two different places.

…a, toggle editor read-only, and destroy editor.
@pomek pomek added the squad:platform Issue to be handled by the Platform team. label Nov 22, 2021
Copy link
Contributor

@psmyrek psmyrek left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

I briefly scanned the CKEditor 5 Inspector code (as this was the first time I saw it), then I ran this PR and it works as expected.

The only one thing to consider is updating the screenshot of the Inspector in the docs after releasing new version: https://ckeditor.com/docs/ckeditor5/latest/framework/guides/development-tools.html#ckeditor-5-inspector. I don't know if it appears elsewhere in the docs.

Copy link
Member

@przemyslaw-zan przemyslaw-zan left a comment

Choose a reason for hiding this comment

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

Should the editor toolbar stay after destroying it? This happens for me for all three editors in the sample.
image

src/commands/commandinspector.js Outdated Show resolved Hide resolved
@oleq
Copy link
Member Author

oleq commented Nov 30, 2021

Should the editor toolbar stay after destroying it? This happens for me for all three editors in the sample.

This is how decoupled document editor works. The integrator takes care of injecting the toolbar and removing it. It's just a sample so I didn't bother.

@oleq
Copy link
Member Author

oleq commented Nov 30, 2021

The only one thing to consider is updating the screenshot of the Inspector in the docs after releasing new version: https://ckeditor.com/docs/ckeditor5/latest/framework/guides/development-tools.html#ckeditor-5-inspector. I don't know if it appears elsewhere in the docs.

README.md of the inspector on Github/Npm, that's for sure.

@pomek
Copy link
Member

pomek commented Nov 30, 2021

I extracted updating documentation to a new issue – #133.

Copy link
Member

@pomek pomek left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@przemyslaw-zan przemyslaw-zan left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squad:platform Issue to be handled by the Platform team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A simple button to get data Toggle editor read-only state
4 participants