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

[#32] Avatar component #46

Merged
merged 13 commits into from
Jan 17, 2024
Merged

[#32] Avatar component #46

merged 13 commits into from
Jan 17, 2024

Conversation

daisy142
Copy link
Contributor

@daisy142 daisy142 commented Jan 8, 2024

What this PR does / why we need it:

  • Add avatar component from ParkUI
  • Update and custom styles for avatar component

Special notes for your reviewer:

Which issue(s) this PR fixes:

Fixes #32

Screenshoots:
Screenshot 2024-01-08 at 12 04 50

Checklist:

  • Added relevant tests or not required
  • Didn't break anything

Copy link
Contributor

@vitran12 vitran12 left a comment

Choose a reason for hiding this comment

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

Thank you for your hard working.
I left a few comment.

import { forwardRef } from 'react';
import { avatar, type AvatarVariantProps } from '@/styled/recipes';

export interface AvatarProps extends ArkAvatarProps, AvatarVariantProps {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should new file name: types.ts and move interface and type into it.

import { avatar, type AvatarVariantProps } from '@/styled/recipes';

export interface AvatarProps extends ArkAvatarProps, AvatarVariantProps {
name?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

You should note comment for name property. example:

/** name is plalala */

src?: string;
}

export const Avatar = forwardRef<HTMLDivElement, AvatarProps>((props, ref) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should write a description for the function and param of us. Example:

/**
 * @description This function render style for Avatar.
 */

</svg>
);

const getInitials = (name = '') =>
Copy link
Contributor

Choose a reason for hiding this comment

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

What is a util?
If this is a util, you can move it into util file


Avatar.displayName = 'Avatar';

const UserIcon = () => (
Copy link
Contributor

Choose a reason for hiding this comment

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

You should we will create 1 file manage Icons.
Example

Import {UserIcon} from 'icons'.

Refactor file structure
To divide icons into seperate folder
@daisy142
Copy link
Contributor Author

Thank you for your hard working. I left a few comment.

I updated. Please help me check it before I update others. Thanks a lot @vitran12

Base automatically changed from feat/alice/new-storybook to main January 16, 2024 10:46
daisy142 and others added 2 commits January 17, 2024 09:15
* Add storybook

* Add mdx file

* Add storybook

* Add mdx file

* Add descriptions

* Add descriptions

* Update descriptions
@daisy142 daisy142 merged commit 388529f into main Jan 17, 2024
2 checks passed
@daisy142 daisy142 deleted the feat/daisy/avatar-component branch January 17, 2024 02:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Avatar
2 participants