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): added size variations #4648

Merged
merged 7 commits into from
Feb 9, 2022
Merged

Conversation

mcoker
Copy link
Contributor

@mcoker mcoker commented Jan 29, 2022

fixes #4647

--

this also adds back a missing page demo template file that was throwing hbs errors on a rebase.

@patternfly-build
Copy link

patternfly-build commented Jan 29, 2022

Copy link
Member

@mcarrano mcarrano 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 doing this so quickly @mcoker . Now that I'm seeing this, I wonder if the small avatar is really useful as I don't think you can fit a recognizable image there.

@mceledonia what if we got rid of the 16px variant and made 24px the smallest and bumped everything else. So XL would then be 128px?

Copy link
Member

@srambach srambach left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@mattnolting mattnolting left a comment

Choose a reason for hiding this comment

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

L🍕TM

@@ -27,4 +40,11 @@
--pf-c-avatar--BorderColor: var(--pf-c-avatar--m-dark--BorderColor);
--pf-c-avatar--BorderWidth: var(--pf-c-avatar--m-dark--BorderWidth);
}

@each $size in $sizes {
Copy link
Contributor

Choose a reason for hiding this comment

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

😎

Comment on lines 45 to 49
### Extra extra large
```hbs
{{> avatar avatar--modifier="pf-m-2xl" avatar--attribute='src="/assets/images/img_avatar-light.svg" alt="Avatar image extra extra large"'}}
```

Copy link
Member

Choose a reason for hiding this comment

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

I think this isn't needed anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated! thanks for spotting that.

Comment on lines 60 to 63
| `.pf-m-sm` | `.pf-c-avatar` | Modifies an avatar to be small. |
| `.pf-m-md` | `.pf-c-avatar` | Modifies an avatar to be medium. |
| `.pf-m-lg` | `.pf-c-avatar` | Modifies an avatar to be large. **Note:** This is the default size. |
| `.pf-m-xl` | `.pf-c-avatar` | Modifies an avatar to be extra large. |
Copy link
Member

Choose a reason for hiding this comment

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

Should we include "at the optional breakpoint" since we're generating all those classes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep! updated with your new link.

@mcarrano
Copy link
Member

mcarrano commented Feb 9, 2022

This looks great @mcoker . My only issue is that you say in the documentation that pf-m-lg is the default size. But it looks like medium is actually the default size. Maybe just an artifact of our bumping up the sizes?

@mcoker
Copy link
Contributor Author

mcoker commented Feb 9, 2022

@mcarrano nice catch, updated!

Copy link
Member

@srambach srambach left a comment

Choose a reason for hiding this comment

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

🎉

Copy link
Member

@mcarrano mcarrano left a comment

Choose a reason for hiding this comment

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

Looks great!

@mcoker mcoker merged commit b07e105 into patternfly:main Feb 9, 2022
@patternfly-build
Copy link

🎉 This PR is included in version 4.177.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

Avatar - support for different sizes
5 participants