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

Allow icon prop to be specified by name #139

Merged
merged 2 commits into from
Nov 7, 2024

Conversation

8845musign
Copy link
Collaborator

@8845musign 8845musign commented Nov 1, 2024

Changes

  • allow icon prop to be specified by name
  • Button / LinkButton / LinkCard

Button

スクリーンショット 2024-11-01 14 04 04

LinkButton

スクリーンショット 2024-11-01 14 04 11

LinkCard

スクリーンショット 2024-11-01 14 05 39

Check

No change in rendering results

  • [-] Browser verification (minimum) Android Chrome/iOS Safari(375px-)
  • [-] CSS not affected by inheritance
  • [-] Layout does not break even if there is an overflow
  • [-] Layout does not break when wraps
  • [-] Added new Component
    • Added data-* prop and id prop
  • [-] Updated Ubie Vitals or Added an update issue(if needed)

Signed-off-by: hiroki.yokouchi <hiroki.yokouchi@dr-ubie.com>
@8845musign 8845musign requested a review from takanorip as a code owner November 1, 2024 04:49
@8845musign 8845musign self-assigned this Nov 1, 2024
Signed-off-by: hiroki.yokouchi <hiroki.yokouchi@dr-ubie.com>
@@ -34,15 +35,15 @@ export type BaseProps = {
/**
* アイコン
*/
icon?: 'default' | ReactNode;
icon?: 'default' | ReactElement | IconName;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In ReactNode, IconName and 'default' are combined because ReactNode contains strings.
Changed to ReactElement because we do not plan to display just strings, numbers, etc.

@takanorip takanorip merged commit 3544a30 into ubie-oss:main Nov 7, 2024
2 checks passed
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