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

refactor: migrate listed components to typescript #6909

Merged
merged 6 commits into from
Jul 16, 2022

Conversation

didoshotev
Copy link
Contributor

Description

migrate listed components:
src/components/PageHero.js
src/components/SkipLink.js

In PageHero content prop, I have set it to any, since I don't see any TS interface for this type of data. This can be changed once we have defined interface.

Please let me know, if you want to separate SkipLinkAnchor into different file, as there are 2 components in the same file (SkipLink) currently.

Related Issue

Refs: #6392

@gatsby-cloud
Copy link

gatsby-cloud bot commented Jul 4, 2022

Gatsby Cloud Build Report

ethereum-org-website-dev

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 12m

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.

Thanks for the PR @didoshotev! left one comment.

isReverse = false,
children,
className,
}) => {
const { buttons, title, header, subtitle, image, alt } = content
Copy link
Member

Choose a reason for hiding this comment

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

For now, we could add the content interface in this file with this structure. Could be something like:

interface IButton {
  path: string
  pathId: string
}

interface IContent {
  buttons?: Array<IButton>
  title: string
  header: string
  subtitle: string
  image: string
  alt: string
}

@didoshotev
Copy link
Contributor Author

@pettinarip I have made the requested changes and pulled the new changes from dev. Let me know if something else needs to be added for this PR.

@github-actions github-actions bot added the content 🖋️ This involves copy additions or edits label Jul 16, 2022
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.

Thanks @didoshotev. I've pushed a few minor things but overall it looks great! thanks for applying the suggestions.

@pettinarip pettinarip merged commit 72e115e into ethereum:dev Jul 16, 2022
@corwintines corwintines mentioned this pull request Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content 🖋️ This involves copy additions or edits
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants