test(storybook): test added for headings #311

sanketshevkar · 2021-03-20T11:23:13Z

Signed-off-by: User shevkar.sanket@gmail.com

Changes

Added cypress test for: Placing cursor in paragraph and changing to header 1, 2, 3. Just wanted the maintainers to review the format and code quality. Once approved I'll add other tests for Headings.

Related Issues

Issue User Interaction Testing #14

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

Signed-off-by: User <shevkar.sanket@gmail.com>

jolanglinais · 2021-03-22T15:46:58Z

This looks like a great start, I'll try to review it as soon as I can.

Cronus1007 · 2021-03-23T13:50:08Z

@sanketshevkar How you have selected the text since cypress doesn't have any sort of api to select the text.The first need is to write the complete selection command. @irmerk @mttrbrts I have written a little bit of code to select the text will need a little bit help in it i guess.

sanketshevkar · 2021-03-23T14:02:40Z

@sanketshevkar How you have selected the text since cypress doesn't have any sort of api to select the text.The first need is to write the complete selection command. @irmerk @mttrbrts I have written a little bit of code to select the text will need a little bit help in it i guess.

Nope. I had seen some scripts online, which could do that. I'm focusing on other tests rn as they seem to be easy and can be completed asap. If you're able get any solution do tell.

Cronus1007 · 2021-03-23T14:08:25Z

@sanketshevkar Plzzz have a look upon it. I am getting that all the tests are failing.

cd packages/storybook
npm run test:e2e

jolanglinais · 2021-03-23T14:30:43Z

packages/storybook/cypress/storybook/markdownEditorHeader.spec.js

+      //Finds the paragraph and place cursor
+      getIframeBody().find("#ap-rich-text-editor > p:nth-child(2)").click();
+      //Find heading dropdown and select heading-3
+      getIframeBody().find("#ap-rich-text-editor-toolbar > div.ui.simple.dropdown").click();
+      getIframeBody().find("#ap-rich-text-editor-toolbar > div.ui.active.visible.simple.dropdown > div.menu.transition.visible > div:nth-child(4)").click();
+      //checks if para changed to heading-3
+      getIframeBody().find("#This-is-text-This-is-italic-text-This-is-bold-text-This-is-a-undefined-This-is-inline-code").should('have.css', 'font-size', '16px', 'font-weight', 'bold');
+      //undo and check
+      getIframeBody().find("#ap-rich-text-editor-toolbar > svg:nth-child(11)").click();
+      getIframeBody().find("#ap-rich-text-editor > p:nth-child(2)").should('exist');
+      //redo and check
+      getIframeBody().find("#ap-rich-text-editor-toolbar > svg:nth-child(12)").click();
+      getIframeBody().find("#This-is-text-This-is-italic-text-This-is-bold-text-This-is-a-undefined-This-is-inline-code").should('have.css', 'font-size', '16px', 'font-weight', 'bold');
+     });


Some initial thoughts... (I'll still do more looking into this whole PR)

Maybe we make some constants at the top of the file to describe what a lot of these are? For example:

const BUTTON_UNDO = 'svg:nth-child(11)'; const ID_TOOLBAR = '#ap-rich-text-editor-toolbar'; // ... etc

Could potential make this more readable.

Similarly, could make a new line for each action:

getIframeBody() .find("#ap-rich-text-editor > p:nth-child(2)") .should('exist');

Not sure I understand the // undo and check part. When undoing, we check to see if the paragraph exists? Is this checking to see if it is a p and no longer has the heading properties?

Yup I wanted to check whether it is a paragraph or not. Should I stick to css checking as I did for headers?

I'll make the code format changes and commit the code.

Thanks for the review @irmerk .

sanketshevkar · 2021-03-23T14:50:04Z

@sanketshevkar Plzzz have a look upon it. I am getting that all the tests are failing.
cd packages/storybook

npm run test:e2e

I'll look into it

jolanglinais · 2021-04-02T15:25:22Z

I think this looks amazing! Looks like some of these tests may be currently failing? Can you resolve and then I'll add more reviewers on this?

sanketshevkar · 2021-04-02T18:30:51Z

I think this looks amazing! Looks like some of these tests may be currently failing? Can you resolve and then I'll add more reviewers on this?

I guess it would have timed out. Because these tests were passing on local repository.

Cronus1007

@sanketshevkarm Assertion Error has occured since the it has not found out the element.Check the Details of the failed tests.

sanketshevkar · 2021-04-02T18:36:38Z

@sanketshevkarm Assertion Error has occured since the it has not found out the element.Check the Details of the failed tests.

It occurs when the storybook fails to load on the browser and thus element is not found. It works well locally.

Signed-off-by: User <shevkar.sanket@gmail.com>

sanketshevkar · 2021-04-05T12:27:31Z

@irmerk this issue here is that the size of headings is being different in different browsers. My local browser for the test is expecting 25px, 20px, 16px for heading 1, 2, 3 respectively. Whereas the ci/cd server expects 25px, 25px, 20px for headings 1, 2, 3 respectively. So if the test are passing on ci-cd server, it is failing on my local machine and vice-versa. I guess we'll have to normalise font-size css for headings. Would this be the right thing to do?

jolanglinais · 2021-04-06T14:55:13Z

I think we should be testing what is in ui-markdown-editor/src/utilities/constants, which looks like is what the ci/cd server is expecting?

Maybe even import those values into the test so we know we're testing what is the source of truth for the CSS.

sanketshevkar · 2021-04-14T11:00:58Z

@irmerk I'm really sorry for the late response.
I tried importing css values from ui-markdown-editor/src/utilities/constants and it's working. Thank you for the suggestion.
The problem was that with merging PR #302, the sizes of H1 and H2 are now same, the difference being their alignment. And this commit wasn't present in my local repository, so the tests were failing.

Also, I think styling for paragraph should be added in ui-markdown-editor/src/utilities/constants, because in ui-markdown-editor/src/utilities/constants style for paragraph is null.

jolanglinais · 2021-05-05T15:54:57Z

@sanketshevkar I believe @Michael-Grover will need to provide context on styling in relation to testing/consistency.

Michael-Grover · 2021-05-06T02:43:40Z

@irmerk @sanketshevkar what is the specific design question here? I checked the heading CSS here and on storybook and it looks consistent with what I specified when I chose the styling for these headings. I think the file linked above is the closest thing to a source of truth I can find. All of the font sizes, colors, alignment, capitalization, etc. is what I expected.

Signed-off-by: User <shevkar.sanket@gmail.com>

sanketshevkar · 2021-05-09T07:08:08Z

@irmerk @Michael-Grover I've imported the CSS constants and used them in the test as the source of truth. Requesting a review for the latest commit.
Thank You.

Michael-Grover · 2021-05-10T16:42:58Z

@sanketshevkar

I took a look at the Netlify preview and it looks like there are only 3 header styles in the dropdown now

And it looks like the style for H6 changed

sanketshevkar · 2021-06-01T20:27:57Z

Continued on PR #350. Had some merge conflicts on my fork and local repo. Sorry for the inconvenience.

test(storybook): test added for headings

7af2a03

Signed-off-by: User <shevkar.sanket@gmail.com>

jolanglinais assigned sanketshevkar Mar 22, 2021

jolanglinais added Difficulty: Medium Package: Markdown Editor ⌨️ Type: Tests 🧪 Test coverage labels Mar 22, 2021

jolanglinais added this to the Markdown Editor: Testing UI milestone Mar 22, 2021

jolanglinais requested review from jolanglinais and DianaLease March 22, 2021 15:46

Cronus1007 mentioned this pull request Mar 23, 2021

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

Closed

9 tasks

jolanglinais reviewed Mar 23, 2021

View reviewed changes

Cronus1007 reviewed Apr 2, 2021

View reviewed changes

sanketshevkar added 2 commits April 5, 2021 17:18

test(storybook): test added for headings

20c5b9d

Signed-off-by: User <shevkar.sanket@gmail.com>

test(storybook): test added for headings

997295b

Signed-off-by: User <shevkar.sanket@gmail.com>

feat(storybook): CSS constansts used for testing

77269c4

Signed-off-by: User <shevkar.sanket@gmail.com>

Merge branch 'accordproject:master' into sanketshevkar/14/headingsTest

ec5ff06

sanketshevkar mentioned this pull request Jun 1, 2021

test(storybook): test headers by placing cursor - #14 #350

Open

9 tasks

sanketshevkar closed this Jun 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(storybook): test added for headings #311

test(storybook): test added for headings #311

sanketshevkar commented Mar 20, 2021 •

edited

Loading

jolanglinais commented Mar 22, 2021

Cronus1007 commented Mar 23, 2021 •

edited

Loading

sanketshevkar commented Mar 23, 2021

Cronus1007 commented Mar 23, 2021 •

edited

Loading

jolanglinais Mar 23, 2021

sanketshevkar Mar 23, 2021

sanketshevkar commented Mar 23, 2021

jolanglinais commented Apr 2, 2021

sanketshevkar commented Apr 2, 2021

Cronus1007 left a comment •

edited

Loading

sanketshevkar commented Apr 2, 2021

sanketshevkar commented Apr 5, 2021

jolanglinais commented Apr 6, 2021

sanketshevkar commented Apr 14, 2021 •

edited

Loading

jolanglinais commented May 5, 2021

Michael-Grover commented May 6, 2021

sanketshevkar commented May 9, 2021

Michael-Grover commented May 10, 2021

sanketshevkar commented Jun 1, 2021

test(storybook): test added for headings #311

test(storybook): test added for headings #311

Conversation

sanketshevkar commented Mar 20, 2021 • edited Loading

Changes

Related Issues

Author Checklist

jolanglinais commented Mar 22, 2021

Cronus1007 commented Mar 23, 2021 • edited Loading

sanketshevkar commented Mar 23, 2021

Cronus1007 commented Mar 23, 2021 • edited Loading

jolanglinais Mar 23, 2021

Choose a reason for hiding this comment

sanketshevkar Mar 23, 2021

Choose a reason for hiding this comment

sanketshevkar commented Mar 23, 2021

jolanglinais commented Apr 2, 2021

sanketshevkar commented Apr 2, 2021

Cronus1007 left a comment • edited Loading

Choose a reason for hiding this comment

sanketshevkar commented Apr 2, 2021

sanketshevkar commented Apr 5, 2021

jolanglinais commented Apr 6, 2021

sanketshevkar commented Apr 14, 2021 • edited Loading

jolanglinais commented May 5, 2021

Michael-Grover commented May 6, 2021

sanketshevkar commented May 9, 2021

Michael-Grover commented May 10, 2021

sanketshevkar commented Jun 1, 2021

sanketshevkar commented Mar 20, 2021 •

edited

Loading

Cronus1007 commented Mar 23, 2021 •

edited

Loading

Cronus1007 commented Mar 23, 2021 •

edited

Loading

Cronus1007 left a comment •

edited

Loading

sanketshevkar commented Apr 14, 2021 •

edited

Loading