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

Disabling table selection should collapse selection #6430

Closed
scofalik opened this issue Mar 13, 2020 · 5 comments · Fixed by ckeditor/ckeditor5-table#280
Closed

Disabling table selection should collapse selection #6430

scofalik opened this issue Mar 13, 2020 · 5 comments · Fixed by ckeditor/ckeditor5-table#280
Assignees
Labels
package:table type:bug This issue reports a buggy (incorrect) behavior.

Comments

@scofalik
Copy link
Contributor

📝 Provide a description of the improvement

At this moment, when you disable table selection plugin, the selection stays as it was, that is, it can be done on multiple cells. This is in my opinion wrong - if I disable the multi-cell selection I don't want it to be existing in the editor anymore.


If you'd like to see this improvement implemented, add a 👍 reaction to this post.

@scofalik
Copy link
Contributor Author

Unfortunately, when I do this:

tableSelectionPlugin.forceDisabled( 'trackChanges' );

editor.model.change( writer => {
	const range = editor.model.schema.getNearestSelectionRange( editor.model.document.selection.getFirstPosition() );

	writer.setSelection( range );
} );

The range is still collapsed on a cell, instead of inside :|.

@mlewand mlewand added the type:improvement This issue reports a possible enhancement of an existing feature. label Mar 13, 2020
@mlewand mlewand added this to the iteration 30 milestone Mar 13, 2020
@mlewand
Copy link
Contributor

mlewand commented Mar 13, 2020

Yea, I guess you need to do some extra magic to shrink to inside of a first/last table cell, as the selection is always outside of a cell.

@scofalik
Copy link
Contributor Author

I've used this and it seems to work:

editor.model.change( writer => {
	let position = editor.model.document.selection.getFirstPosition();

	if ( position.nodeAfter && position.nodeAfter.is( 'tableCell' ) ) {
		position = writer.createPositionAt( position.nodeAfter, 0 );
	}

	const range = editor.model.schema.getNearestSelectionRange( position );

	writer.setSelection( range );
} );

But I treat it as a hack and looking foward to having it inside the plugin.

@scofalik
Copy link
Contributor Author

It seems to work even for more difficult cases like this:

(selection ends up on an image) or an empty cell.

@mlewand
Copy link
Contributor

mlewand commented Mar 13, 2020

We were able to make a temp workaorund in CF, since we need to start testing iteration 30 ASAP I'm moving this to it. 31st

@mlewand mlewand modified the milestones: iteration 30, iteration 31 Mar 13, 2020
@oleq oleq self-assigned this Mar 13, 2020
@oleq oleq added type:bug This issue reports a buggy (incorrect) behavior. and removed type:improvement This issue reports a possible enhancement of an existing feature. labels Mar 13, 2020
jodator added a commit to ckeditor/ckeditor5-table that referenced this issue Mar 24, 2020
Fix: `TableSelection` plugin should collapse a multi-cell selection when it gets disabled. Closes ckeditor/ckeditor5#6430.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:table type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
3 participants