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

[TS Code Style]Emoji component has a required size props #6626

Closed
Mousticke opened this issue Jun 9, 2022 · 1 comment · Fixed by #6631
Closed

[TS Code Style]Emoji component has a required size props #6626

Mousticke opened this issue Jun 9, 2022 · 1 comment · Fixed by #6631
Assignees
Labels
bug 🐛 Something isn't working

Comments

@Mousticke
Copy link
Contributor

Describe the bug

This is purely coding style issue.
The Emoji component has a size props with a default value

const Emoji: React.FC<IProps> = ({ size = 1.5, text, ...props }) 

with a defined interface IProps

export interface IProps extends MarginProps {
  size: number
  text: string
}

Here size is required but set with a default value in the functional components. Other components require the Emoji component and have to set the size because of the interface.
We should replace the required props as an optional props.

Expected behavior

We expect this interface in src/components/Emoji.tsx

export interface IProps extends MarginProps {
  size?: number
  text: string
}

Additional context

Part of the Typescript rollout: #6392
Emoji PR for the migration: #6540

Want to contribute?

You can assign me this issue if you want.

@Mousticke Mousticke added the bug 🐛 Something isn't working label Jun 9, 2022
Mousticke added a commit to Mousticke/ethereum-org-website that referenced this issue Jun 10, 2022
importing a svg in React cause the typescript compiler to raise an error
"Could not find module...". svg are not React component so we have to
define a module that export it as a React component

fix(emoji): size should be optional

Refs: ethereum#6382 ethereum#6626
@pettinarip
Copy link
Member

Good catch @Mousticke. I've assigned it to you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants