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: add contrib avatars #99

Closed
wants to merge 5 commits into from

Conversation

IgnaceMaes
Copy link
Member

@IgnaceMaes IgnaceMaes commented Sep 30, 2023

This PR is a proposal to add avatars of contributors when mentioning them in the Ember Times.

Why?

  • Currently the section is quite the large orange blob due to all the links, making it hard to read through
  • Ember is driven by the community, making giving credit an important part of the working
  • ...
  • Profit?

Here's a quick photoshop of what the rendered output of this app would look like in the Ember Times. It's a copy paste, so the default styling of the links will be the orange styling in the actual app.
contribs

@IgnaceMaes
Copy link
Member Author

Further discussed here: https://discord.com/channels/480462759797063690/1157672350154764288/1158027322276970567

Tweaks:

  • Smaller avaters so it doesn't take up too big of a portion of the blog post
  • Only show the display name so the text stays in balance with the image

Here is what it looks like in the real ember-blog app:

image

MinThaMie
MinThaMie previously approved these changes Oct 26, 2023
Copy link
Contributor

@MinThaMie MinThaMie left a comment

Choose a reason for hiding this comment

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

This is sooooo cool :)

@IgnaceMaes
Copy link
Member Author

Thanks for considering this!

Full disclosure, and important to review when deciding on this:

  • The image assets are hosted by GitHub and not by the own app. That does imply if GitHub ever changes the urls these could break. However, they seem like permalinks? e.g. https://avatars.githubusercontent.com/u/10243652?v=4. So they should remain stable in theory.
  • Some grid CSS features don't work on IE11, but overall still has good browser support. So I don't think that's considered an issue anymore these days?

@mansona
Copy link
Member

mansona commented Oct 26, 2023

Hey 👋 we discussed this in the #st-website weekly meeting today.

We like the idea but we think we could make some improvements:

  • can we try round avatars? i.e. more similar to what we have in the teams page https://emberjs.com/teams/ (or you could try a squircle 😂)
  • we need to figure out some solution to the over-wrapping of the names
  • we need to make sure we have github handles still in there

Lukas had a good idea to try putting the names to the right hand side of images and having fewer on each line but then we could have name and github handles above each other. That way we could have overflow hidden with elipsis and hover-state for the full name and a fixed width

I'm also a bit uncomfortable with using the direct github URLs, but we can make this look good and come up with a plan for that after we're ready to launch 👍

@IgnaceMaes
Copy link
Member Author

I like aligning the name to the right of the avatar, it seems like the only way if we want to have both full name and handle, plus the image.

Here's both options:

Rounded square Circle
image image2

Personally I prefer the first option as it feels more like a rigid grid. But no strong opinion.

@IgnaceMaes
Copy link
Member Author

Regarding linking to the asset on GitHub, I think theres a couple of ways forward:

  1. We download the images and store the assets as image files somewhere. This seems complicated and not a quick fix. It also breaks the flow by copy pasting this output to the Ember Times as there's now also assets in play.
  2. We inline the assets as base64 encoded. I prototyped this here: feat: Inline avatars as base64 IgnaceMaes/whats-new-in-emberland#1. It works and solves the concern of an external host 🎉 But on the flipside: the markdown will become more bloated.
  3. We stick with linking to GitHub. Considering this endpoint structure hasn't changed in 10 years it doesn't seem likely that it will in the near future.

@MinThaMie
Copy link
Contributor

Sorry for having such a long name what it breaks the flow :P I also like the rounded square better. I would maybe omit the ( for the username

@IgnaceMaes
Copy link
Member Author

Thank you for providing an edge case before it's a bug in production 😄

I used the brackets for consistency with the guidelines in the blog post itself:

add the contributor in the post in format "FirstName LastName (@githubUserName)" linked to their GitHub account

I kinda like it 😬

@IgnaceMaes
Copy link
Member Author

Giving this another thought: it would probably break our Times email letter as it uses CSS grid which is not supported in a majority of clients.

A proper solution would likely mean implementing it using tables.

I'm going to close this as I simply do not have the priority to tackle this atm. But I'm open to review if someone else wants to pick this up.

@IgnaceMaes IgnaceMaes closed this Apr 16, 2024
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.

3 participants