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.

Sign up for GitHub

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

Jump to bottom

Clear highlights and pointer on BlockManager even if selection was inside of the editor before clicking outside #2644

Open
wants to merge 3 commits into
base: next
Choose a base branch
from
Open

Clear highlights and pointer on BlockManager even if selection was inside of the editor before clicking outside #2644

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1701bcb
Clear highlights and pointer on BlockManager even if selection was in…
VikhorKonstantin Feb 29, 2024
63189a3
commented isTrusted check because this might break tests - isTrusted …
VikhorKonstantin Mar 1, 2024
9ed922b
added test verifying that we're not opening toolbars on '/' if user c…
VikhorKonstantin Mar 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/components/modules/ui.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,16 +611,18 @@ export default class UI extends Module<UINodes> {
/**
* Sometimes we emulate click on some UI elements, for example by Enter on Block Settings button
* We don't need to handle such events, because they handled in other place.
*
* PS: Commented this out because it makes this method behave differently in tests. Cypress-generated clicks are having isTrusted == false.
*/
if (!event.isTrusted) {
return;
}
// if (!event.isTrusted) {
// return;
// }
/**
* Close Inline Toolbar when nothing selected
* Do not fire check on clicks at the Inline Toolbar buttons
*/
const target = event.target as HTMLElement;
const clickedInsideOfEditor = this.nodes.holder.contains(target) || Selection.isAtEditor;
Copy link
Member

Choose a reason for hiding this comment

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

I can't remember why it was added. Anyway, I've tested it and it looks like everything is ok. So please add a test (at the Ui.cy.ts) and a change log

Copy link
Contributor Author

@VikhorKonstantin VikhorKonstantin Mar 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@neSpecc, I just tried to write some tests and encountered an issue with the ui.ts:documentClicked function:
image
It appears that this check causes clicks made by Cypress click() to behave differently from user-generated clicks (because Cypress-generated clicks will always have isTrusted == false). I'm concerned that this might break other tests as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@neSpecc, I've commented this out with an explanation in a comment. Seems like we're not triggering Click events anywhere in the code so it shouldn't break anything.

Copy link
Member

Choose a reason for hiding this comment

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

I've found a bug with Block Tunes pressing by Enter. It sometimes removes the block itself (like Enter pressed when block is selected). It can be reproduces in the next so I'm gonna dig in it soon.

They can be related

Copy link
Member

Choose a reason for hiding this comment

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

Hey @VikhorKonstantin. I've resolved several problems with Enter press. Please, pull the next and check if the problem still actual

const clickedInsideOfEditor = this.nodes.holder.contains(target);

if (!clickedInsideOfEditor) {
/**
Expand Down
53 changes: 53 additions & 0 deletions test/cypress/tests/modules/Ui.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,5 +92,58 @@ describe('Ui module', function () {
});
});
});

describe('Clicking outside', function () {
it('should clear current block even if selection was inside the editor before clicking', function () {
cy.createEditor({
data: {
blocks: [
{
id: 'block1',
type: 'paragraph',
data: {
text: '',
},
},
],
},
}).as('editorInstance');

cy.get('[data-cy=editorjs]')
.then(editor => {
const editorsParent = editor[0].parentNode;
const input = editorsParent.ownerDocument.createElement('div');

input.contentEditable = 'true';
input.style.width = '20px';
input.style.height = '20px';
input.setAttribute('data-cy', 'test-input');

editorsParent.appendChild(input);
});

/**
* Put cursor inside the editor
*/
cy.get('[data-cy=editorjs]')
.find('.ce-paragraph')
.first()
.click();

/**
* Click outside of the editor and type '/'
*/
cy.get('[data-cy=test-input]')
.click()
.type('/');

/**
* Toolbox shouldn't be open
*/
cy.get('[data-cy=editorjs]')
.get('div.ce-toolbox .ce-popover')
.should('not.be.visible');
});
});
});
});
Loading