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

Create an avatar tag to be usable anywhere we need an avatar. #1298

Merged
merged 4 commits into from
Sep 27, 2024

Conversation

brianjp93
Copy link
Collaborator

@brianjp93
Copy link
Collaborator Author

I did this using a partial + a django tag so the avatar can be created using just a django tag. I'm also open to using the partial with a surrounding with to set variables but I thought the tag is easier to use and harder to mess up.

@brianjp93
Copy link
Collaborator Author

@rbbeeston Updated header avatar and avatar on /users/me/

- fixes boostorg#1285
- Use a django tag to render an avatar component.
@vinniefalco
Copy link
Member

I don't know what a "django tag" is, but I like the sound of it

<div class="w-8 h-8 rounded bg-gray-300 mx-auto"></div>
{% endif %}
<div class="flex flex-col gap-y-2 w-20 items-center">
{% avatar name=author.name image_url=author.avatar_url href=author.github_profile_url %}
Copy link
Member

@vinniefalco vinniefalco Sep 27, 2024

Choose a reason for hiding this comment

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

This is a django tag? It looks just like a partial.. what am I missing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's similar to a partial.

  1. The way that it is created is slightly different.
{% include "partials/avatar.html" with name="" image_url="" href="" %}
  1. Using a django tag ensures that the required fields are provided when using it. If we use the include method instead, and don't include a name for example, it will still render but without a name.

I opted for the django tag as it's slightly easier to use but I'm fine with either way.

<div class="w-8 h-8 rounded bg-silver">
</div>
{% endif %}
{% avatar name=item.commit__author__name image_url=item.commit__author__avatar_url href=None %}
Copy link
Member

Choose a reason for hiding this comment

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

Why does "href" have different values here versus the previous one? Why is image_url different here from the previous one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because the object is different. Each item in this queryset is actually a LibraryVersion with annotations to count commits by commit author per version.

@vinniefalco
Copy link
Member

This is certainly an improvement, and I think it could be better still. I would like to not have to specify name, image, and href individually. Instead I would like the avatar to be passed a single "object" with the appropriate keys. And delegate to the django code filling in that object. This is just my intuition, I could be wrong....

@brianjp93
Copy link
Collaborator Author

@vinniefalco I want the ability to specify those fields individually but I will create an avatar that is specifically for rendering from a user object, that may be what you want.

@vinniefalco
Copy link
Member

This PR is fine as-is, and if you want to add rendering from a user object that is OK too and it being in a separate PR is also ok.

@brianjp93 brianjp93 merged commit 45a3fa7 into boostorg:develop Sep 27, 2024
4 checks passed
@brianjp93 brianjp93 deleted the 1285-avatar-partial branch September 27, 2024 22:30
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.

Create partial for avatars
3 participants