tests(storybook): adding selection commands for text #313

Cronus1007 · 2021-03-23T14:27:34Z

Stale PR for discussion

Related Issues

Issue User Interaction Testing #14
Pull Request test(storybook): test added for headings #311

Author Checklist

Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
Vital features and changes captured in unit and/or integration tests
Commits messages follow AP format
Extend the documentation, if necessary
Merging to master from fork:branchname
Manual accessibility test performed
- Keyboard-only access, including forms
- Contrast at least WCAG Level A
- Appropriate labels, alt text, and instructions

fix(ui-markdown-editor): text deselection - accordproject#291 (accordproject#292)

fix(ui-markdown-editor): handle cut image operation - accordproject#294 (accordproject#298)

bump

nkkk

Signed-off-by: Cronus1007 <[email protected]>

Cronus1007 · 2021-03-23T14:30:17Z

@irmerk @mttrbrts @DianaLease The biggest problem here is referring to Text Node. I used createTreeWalker to refer to text Node but it is not working as I suppose it to work.

mttrbrts · 2021-03-24T11:35:51Z

Thanks @Cronus1007 we're in the process of migrating the CI pipeline which affects how these tests are run. Please be patient while we complete this work.

I used createTreeWalker to refer to text Node but it is not working as I suppose it to work.

Can you describe the issue in some more detail, please?

Cronus1007 · 2021-03-24T11:55:15Z

@mttrbrts Since cypress doesn't have any api to provide selection of text required in the e2e testing of web-components library. So here I have written commands to select the text but the problem that arises here that I am unable to detect Text Node. It is easy to detect Image Nodes like this

const [imageNode] = Editor.nodes(editor, { match: n => n.type === 'image' });

We need a similair way to detect Text Nodes as well in Slate .

mttrbrts · 2021-03-24T12:02:37Z

@mttrbrts Since cypress doesn't have any api to provide selection of text required in the e2e testing of web-components library. So here I have written commands to select the text but the problem that arises here that I am unable to detect Text Node. It is easy to detect Image Nodes like this
const [imageNode] = Editor.nodes(editor, { match: n => n.type === 'image' });
We need a similair way to detect Text Nodes as well in Slate .

Got it, thanks @Cronus1007. I'll review and get back to you. Unless @dselman @irmerk or @DianaLease has an immediate solution?

sanketshevkar

@Cronus1007 can you describe the role of getTextNodeNode function and createTreeWalker function?

Cronus1007 · 2021-03-29T15:39:05Z

@sanketshevkar

Since Slate document is a nested and recursive structure so the first data structure that comes to mind is tree and for representing a text A Text Node must be there so it is important to getTextNode and should be the root node.
Second createTreeWalker is a function which gets this node and parses the text from beginning node to the end node.

sanketshevkar · 2021-03-29T16:15:14Z

@Cronus1007
While testing I found out that el in the getTextNode function stores the whole body as an object within the storybook iframe instead of just the text node object that you want to select. Is this the same behaviour you are expecting?

Cronus1007 · 2021-03-30T09:17:15Z

@sanketshevkar Here createTreeWalker function requires their first argument as node.But the $el refers to the text so it is required to convert the text into node.

jolanglinais · 2021-04-02T15:37:28Z

Unsure if I'm following, but @Cronus1007 Slate has a number of built in API to identify text. There are some examples throughout the code here in web-components of us using this to look at the text value or attributes.

Cronus1007 · 2021-04-02T18:27:27Z

packages/storybook/cypress/support/commands.js

+Cypress.Commands.add('selection', { prevSubject: true }, (subject, fn) => {
+    cy.wrap(subject)
+      .trigger('mousedown')
+      .then(fn)
+      .trigger('mouseup');
+
+    cy.document().trigger('selectionchange');
+    return cy.wrap(subject);
+  });
+
+  Cypress.Commands.add('setSelection', { prevSubject: true }, (subject, query, endQuery) => {
+    return cy.wrap(subject)
+      .selection($el => {


@irmerk Yes you are on the right path.Here the $el should identify the text as node although I have wrapped a node here in selection custom command.But still it is taking up the text rather than the element.
Here I want to refer to this as well since Slate follows nested and recursive structure but still it is unable to capture node.A problem may be there in selection Custom Command.
Plus @sanketshevkar PR #311 would also fail until we found a way to select the text.

#311 does not require the text to be selected, its about placing the cursor in a para and then changing the headings.

I think I'm not following after all. Why do we need these custom commands when Slate has built in functions for this?

@irmerk I guess you are not on the right track.Since slate has no connection with the selection of text.The Cypress automates that work which a user has to do.The selection of text must be done by Cypress.

jolanglinais · 2021-05-05T16:04:04Z

I am still unfamiliar with Cypress. This PR would need help from @dselman @mttrbrts @rkotangoor

Cronus1007 and others added 5 commits March 12, 2021 16:05

Merge pull request #1 from accordproject/master

4063bde

fix(ui-markdown-editor): text deselection - accordproject#291 (accordproject#292)

Merge pull request #3 from accordproject/master

d921339

fix(ui-markdown-editor): handle cut image operation - accordproject#294 (accordproject#298)

Merge pull request #4 from accordproject/master

1a70996

bump

Merge pull request #5 from accordproject/master

2e189ca

nkkk

tests(storybook): adding selection commands for text

5f548e4

Signed-off-by: Cronus1007 <[email protected]>

Cronus1007 force-pushed the issue-120 branch from 815fc92 to 5f548e4 Compare March 23, 2021 14:28

Cronus1007 changed the title ~~Issue 120~~ tests(storybook): adding selection commands for text Mar 23, 2021

Cronus1007 marked this pull request as draft March 23, 2021 14:31

Cronus1007 marked this pull request as ready for review March 23, 2021 17:20

mttrbrts added Difficulty: Medium Package: Markdown Editor ⌨️ Type: Chore 🧼 labels Mar 24, 2021

mttrbrts requested review from mttrbrts, DianaLease and dselman March 24, 2021 11:36

mttrbrts added Type: Tests 🧪 Test coverage and removed Type: Chore 🧼 labels Mar 24, 2021

sanketshevkar reviewed Mar 29, 2021

View reviewed changes

Cronus1007 commented Apr 2, 2021

View reviewed changes

Cronus1007 requested a review from jolanglinais April 7, 2021 19:35

Cronus1007 closed this May 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests(storybook): adding selection commands for text #313

tests(storybook): adding selection commands for text #313

Cronus1007 commented Mar 23, 2021

Cronus1007 commented Mar 23, 2021 •

edited

Loading

mttrbrts commented Mar 24, 2021

Cronus1007 commented Mar 24, 2021

mttrbrts commented Mar 24, 2021 •

edited

Loading

sanketshevkar left a comment

Cronus1007 commented Mar 29, 2021

sanketshevkar commented Mar 29, 2021

Cronus1007 commented Mar 30, 2021

jolanglinais commented Apr 2, 2021

Cronus1007 Apr 2, 2021 •

edited

Loading

sanketshevkar Apr 2, 2021 •

edited

Loading

jolanglinais Apr 6, 2021

Cronus1007 Apr 6, 2021 •

edited

Loading

jolanglinais commented May 5, 2021

tests(storybook): adding selection commands for text #313

tests(storybook): adding selection commands for text #313

Conversation

Cronus1007 commented Mar 23, 2021

Stale PR for discussion

Related Issues

Author Checklist

Cronus1007 commented Mar 23, 2021 • edited Loading

mttrbrts commented Mar 24, 2021

Cronus1007 commented Mar 24, 2021

mttrbrts commented Mar 24, 2021 • edited Loading

sanketshevkar left a comment

Choose a reason for hiding this comment

Cronus1007 commented Mar 29, 2021

sanketshevkar commented Mar 29, 2021

Cronus1007 commented Mar 30, 2021

jolanglinais commented Apr 2, 2021

Cronus1007 Apr 2, 2021 • edited Loading

Choose a reason for hiding this comment

sanketshevkar Apr 2, 2021 • edited Loading

Choose a reason for hiding this comment

jolanglinais Apr 6, 2021

Choose a reason for hiding this comment

Cronus1007 Apr 6, 2021 • edited Loading

Choose a reason for hiding this comment

jolanglinais commented May 5, 2021

Cronus1007 commented Mar 23, 2021 •

edited

Loading

mttrbrts commented Mar 24, 2021 •

edited

Loading

Cronus1007 Apr 2, 2021 •

edited

Loading

sanketshevkar Apr 2, 2021 •

edited

Loading

Cronus1007 Apr 6, 2021 •

edited

Loading