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

editor is still counted as empty when a blockquote or list is selected #10

Open
alicewriteswrongs opened this issue Jan 7, 2019 · 3 comments
Assignees
Labels

Comments

@alicewriteswrongs
Copy link

alicewriteswrongs commented Jan 7, 2019

Hey! I'm using this library to add some placeholder text my empty editor instance, and mostly it is working great.

A co-worker of mine did notice an issue, however, where the editor is still marked as empty in a few cases where it should not be, particularly because the editor UI starts to show UI indicating that the user has made some input, but because the .ck-editor__is-empty class is still on the editor element my css ::before placeholder still shows up.

This seems to be happening if you click the blockquote or list options on an empty editor. Here's a screenshot of the result:

emptyplaceholder

I'm using a custom build of ckeditor5 based on the classic build. The code is open source, so if you want to look at it it's here: https://github.com/mitodl/ckeditor-custom-build

Let me know if you need any more info!

@alexeckermann alexeckermann self-assigned this Jan 13, 2019
@alexeckermann
Copy link
Owner

Thanks for reporting this.

The issue centres around the question of what does an empty editor look like in its data model. The plugin has an over simplified answer which checks to see if there is any text content in the root elements tree. Since bullet/list blocks have a visual style when they contain no text content this creates what you see above.

I'll have a crack today at seeing if theres some new CK5 engine properties at our disposal to provide a more effective answer to the emptiness question.

alexeckermann added a commit that referenced this issue Jan 14, 2019
@alexeckermann
Copy link
Owner

The current solution in-progress (29db09a) is to ask if the document is visually empty. How that is determined goes like so:

For each child element...

  1. Is the isEmpty property false? If so, break and return false
  2. Is the elements name in the elementNamesWithVisualPresence array? If so, break and return false
  • Otherwise return true.

function isDocumentVisuallyEmpty( document ) {
const root = document.getRoot();
for ( const node of root.getChildren() ) {
if ( node.isEmpty === false ) {
return false;
}
if ( node.is( 'element' ) && isElementVisuallyPresent( node ) ) {
return false;
}
}
return true;
}
const elementNamesWithVisualPresence = [ 'listItem', 'blockQuote' ];
function isElementVisuallyPresent( element ) {
return elementNamesWithVisualPresence.indexOf( element.name ) !== -1;
}

This does add a layer tree walking to the change:data callback. To mitigate this the isDocumentVisuallyEmpty function will check an elements isEmpty property first and break before continuing with more expensive checks and/or more loop iterations.

Before this can be merged in there needs to be some consideration around how new element names are added to the visually present list. By default, we can provide any elements that are in the CK built plugins. But, we will need to have an option for adding additional element names if the developer has custom element plugins in use.

Finally, if there is better terminology to replace "visually present" please make a suggestion. It's the only option that comes to mind at the moment.

@alexeckermann
Copy link
Owner

TODO:

  • Research and update list of element names in official plugins for elementNamesWithVisualPresence
  • Add config API for adding to elementNamesWithVisualPresence list
  • Update README to include definition of 'empty'
  • Ensure testing is up to scratch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants