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

Migrated ActionCard.js to TS #6512

Merged
merged 3 commits into from
May 31, 2022
Merged

Migrated ActionCard.js to TS #6512

merged 3 commits into from
May 31, 2022

Conversation

hursittarcan
Copy link
Contributor

Description

  • Migrated ActionCard component to TypeScript.

Related Issue

@gatsby-cloud
Copy link

gatsby-cloud bot commented May 31, 2022

Gatsby Cloud Build Report

🚩 Your build failed. See the build logs here

Errors

"gatsby-transformer-json" threw an error while running the onCreateNode lifecycle:

Unable to parse JSON: file /usr/src/app/www/src/data/externalTutorials.json

  80 |   } catch {
  81 |     const hint = node.absolutePath ? `file ${node.absolutePath}` : `in node ${node.id}`;
> 82 |     throw new Error(`Unable to parse JSON: ${hint}`);
     |           ^
  83 |   }
  84 |
  85 |   if (_.isArray(parsedContent)) {

Copy link
Member

@pettinarip pettinarip left a comment

Choose a reason for hiding this comment

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

Awesome @hursittarcan thanks for the PR!

Please, check the comment I made.

Also, when I ran yarn type-check I found an error with the <ImageWrapper> component used here. To fix it, you will need to declare the extra props that we are using in this case.

// on line #20
const ImageWrapper = styled.div<{
  isRight: boolean | undefined
  isBottom: boolean | undefined
}>`

After doing that, run again yarn type-check to make sure that everything is good.

className,
isRight,
isBottom = true,
export interface IProps {
Copy link
Member

Choose a reason for hiding this comment

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

You made everything optional but there are some props that we do want to make them as required.
How about this?

export interface IProps {
  to: string
  alt?: string
  image: string
  title: string
  description: string
  className?: string
  isRight?: boolean
  isBottom?: boolean
}

Copy link
Member

@pettinarip pettinarip left a comment

Choose a reason for hiding this comment

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

Looking great! I noticed a misalignment in the format. Can you fix that?

The format should be covered by prettier. Try to do the next:

  1. Make a small format change manually in the file
  2. Make a new commit
  3. prettier should format it automatically when you make a new commit

src/components/ActionCard.tsx Outdated Show resolved Hide resolved
@hursittarcan
Copy link
Contributor Author

Hey @pettinarip,
I believe the formatting is fixed now. I think the extra spaces were because of my IDE.

Copy link
Member

@pettinarip pettinarip left a comment

Choose a reason for hiding this comment

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

💪🏼

@pettinarip pettinarip merged commit a1052fa into ethereum:dev May 31, 2022
@pettinarip
Copy link
Member

@all-contributors please add @hursittarcan for code

@allcontributors
Copy link
Contributor

@pettinarip

I've put up a pull request to add @hursittarcan! 🎉

@wackerow wackerow mentioned this pull request Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants