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

feat(avatar): add disabled state in avatar component #2232

Merged

Conversation

anuradha9712
Copy link
Collaborator

What does this implement/fix? Explain your changes.

...

Does this close any currently open issues?

...

Any other comments?

...

Dependent PRs/Commits

...

Describe breaking changes, if any.

...

Checklist

Check all those that are applicable and complete.

  • Merged with latest master branch

@anuradha9712 anuradha9712 force-pushed the feat-avatar-disabled-state branch from af6189d to 62a3195 Compare June 11, 2024 11:27
@anuradha9712 anuradha9712 marked this pull request as draft June 11, 2024 11:29
@anuradha9712 anuradha9712 self-assigned this Jun 11, 2024
@anuradha9712 anuradha9712 force-pushed the feat-avatar-disabled-state branch from 944738d to a7a140b Compare June 19, 2024 13:12
@anuradha9712 anuradha9712 marked this pull request as ready for review June 19, 2024 13:12
@anuradha9712 anuradha9712 force-pushed the feat-avatar-disabled-state branch from a7a140b to 81e0104 Compare June 19, 2024 13:12
@samyak3009
Copy link
Collaborator

Screenshot 2024-06-20 at 1 04 29 PM please check this once, if i am using a avatar wrapped in a div which is having `ellipsis--noWrap` class then its focus state is breaking.

@@ -72,7 +87,10 @@ export const Avatar = (props: AvatarProps) => {
? children.trim().slice(0, initialsLength)
: `${firstName ? firstName.trim()[0] : ''}${lastName ? lastName.trim()[0] : ''}`;

const tooltip = children && typeof children === 'string' ? children : `${firstName || ''} ${lastName || ''}` || '';
const tooltip =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please create a separate function for this instead of writing conditional code as it look complex to read at first glance.

@anuradha9712
Copy link
Collaborator Author

Screenshot 2024-06-20 at 1 04 29 PM please check this once, if i am using a avatar wrapped in a div which is having ellipsis--noWrap class then its focus state is breaking.

@samyak3009 In case of Avatar been used with Overflow Text wrapped inside div, the ellipsis--noWrap class should only be applied on Text component not on the entire div. Please refer to the following example:

Screenshot 2024-06-20 at 3 45 35 PM

@anuradha9712 anuradha9712 force-pushed the feat-avatar-disabled-state branch from 46f2b24 to 008691a Compare June 20, 2024 10:55
@samyak3009 samyak3009 merged commit c1ef8cd into innovaccer:develop Jun 20, 2024
4 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