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

QUIC-1590-widget-markdown-card #279

Merged
merged 40 commits into from
Jul 28, 2022
Merged

Conversation

johnniedfeldt
Copy link
Contributor

Description

Add support for MarkdownCard

πŸ’β€

Looks like this

image

image

🀨


PR check-list

  • Add Jira ticket number to the PR title or to the commit if the PR only has one commit.
  • Your final merge commit message is in the format <type>: <description> as per conventional commits.
    • Example: fix: add shadow to image QUIC-123

@johnniedfeldt
Copy link
Contributor Author

johnniedfeldt commented Jul 18, 2022

  • Jest unittests having issues with ReactMarkdown

Copy link
Contributor

@allanpope allanpope left a comment

Choose a reason for hiding this comment

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

Nice one, looking pretty good. Just a few comments, yell out if you disagree or have any questions

src/components/ContentCards/ContentCards.tsx Outdated Show resolved Hide resolved
src/components/Heading/Heading.tsx Outdated Show resolved Hide resolved
src/components/MarkdownCard/examples/cheatsheet.md Outdated Show resolved Hide resolved
src/components/MarkdownCard/MarkdownCard.tsx Outdated Show resolved Hide resolved
src/components/Heading/Heading.tsx Outdated Show resolved Hide resolved
src/components/MarkdownCard/MarkdownCard.tsx Outdated Show resolved Hide resolved
src/components/Heading/Heading.tsx Outdated Show resolved Hide resolved
.husky/pre-commit Outdated Show resolved Hide resolved
@johnniedfeldt johnniedfeldt marked this pull request as ready for review July 22, 2022 03:25
@allanpope
Copy link
Contributor

Could you add a cypress test https://github.com/soulmachines/sm-web-component/blob/master/cypress/e2e/content-cards.spec.ts#L7

It tests the full flow, loads a browser with the snippet and enters the command in the input and verifies that a card shows. In your case you'd add the command from the QA test corpus and then just expect a certain bit of text to show up

@johnniedfeldt
Copy link
Contributor Author

@allanpope I've fixed the Cypress issues by updating react-markdown and remark-gfm and creating a mock for each. Now the Jest tests pass but there are a lot of tests we cannot currently run. For now I have commented them out since I figure we will add them back in later when the Jest issue gets resolved. Is this fine or would you like me to remove them completely? Also, I'll see about removing the warnings that causes.

jest.config.js Outdated Show resolved Hide resolved
__mocks__/react-markdown.tsx Outdated Show resolved Hide resolved
src/components/MarkdownCard/MarkdownCard.test.tsx Outdated Show resolved Hide resolved
__mocks__/react-markdown.tsx Outdated Show resolved Hide resolved
cypress/e2e/content-cards.spec.ts Outdated Show resolved Hide resolved
@allanpope
Copy link
Contributor

Looks good, you've just go a merge conflict to fix

@allanpope allanpope merged commit 4778907 into master Jul 28, 2022
@allanpope allanpope deleted the QUIC-1590-widget-markdown-card branch July 28, 2022 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants