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

Showcase GitHub repo link languages #99

Merged

Conversation

HiDeoo
Copy link
Contributor

@HiDeoo HiDeoo commented Jun 1, 2023

Related issues

This pull request is an attempt to fix issue #33.

Description

This pull request is a draft to start a discussion about the issue as I think there are two axes to consider:

  • The fetching and computing part of the data
  • Displaying the data

The first part is implemented with tests in this PR and contains the basic features to support displaying the language stats. At the moment, the following informations are fetch / computed:

  • The language name
  • The GitHub language color
  • The language global percentage

Depending on how this PR goes, some features may even be removed, e.g. if we don't use the percentage or colors.

The second part in this PR is a very basic implementation using badges.

I decided against progress bars as I think the main reason for GitHub to use this format is due to the limited space in the OG images. I think badges are more readable and more in line with the rest of the website.

This basic implementation is also color-less as I think the number of themes makes it difficult to get a good color for all of them considering that GitHub returns only one color and not one per theme they have.

For comparison, in my personal website, I use a library to tweak the colors returned by GitHub to be more readable on my website while still being close to the original colors. I also tweak them differently depending on the theme. I also have some specific overrides for some languages.

This approach works relatively well as I only have 2 themes but I don't think it would scale well for openresource.dev and the current number of themes.

Altho, to be honest, I do not find the current implementation that bad even without colors.

SCR-20230601-rmcy

I tried using the GitHub colors as border colors but I don't think it looks good and it's not really readable on some themes / languages combinations.

SCR-20230601-rmjs

I also tried as background colors but the same issues as above apply.

SCR-20230601-rmrr

Do you have any thoughts / suggestions or even other ideas on how to display the data?

Motivation & Context

This PR adds language stats to the showcase page for GitHub repository links.

Type of changes

  • New feature (non-breaking change which adds functionality)

Checklist

@vercel
Copy link

vercel bot commented Jun 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
main-branch-openresource-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 2, 2023 8:02am

@julien-deramond
Copy link
Member

Wow exciting PR 🤗

I decided against progress bars as I think the main reason for GitHub to use this format is due to the limited space in the OG images. I think badges are more readable and more in line with the rest of the website.

I'm down for it! Even in terms of a11y, should be better.

Regarding the colors, it's nice to see what you tried based on GitHub colors but, as you said, the multi-themes current approach prevents us to find the right matches every time. On top of that, it's fairer if every language is rendered the same way and rather discretely on the page because it's not necessarily the most important information on the project card IMO.

So the current implementation is better IMO than the others in terms of rendering. However, I find the badges too "present" with the Cupcake theme for example, as too colored. I would tend to use the default outline badge from daisyUI even if not as "fun":

(Version's not right but the equivalent of https://daisyui.com/components/badge/#outline-badge)

<div class="badge badge-outline">default</div>

Do you think it makes sense? If not we can keep your actual version and wait for feedback.

Regarding the percentage, I'm not sure it's really useful to display it in the badge itself. Maybe we can just sort the badges so that the language the most used is displayed first. I'm even wondering if we shouldn't remove the languages under xx% but I'm not sure about it.

@HiDeoo
Copy link
Contributor Author

HiDeoo commented Jun 2, 2023

I would tend to use the default outline badge from daisyUI

This is indeed way better, great suggestion. Here is a preview with the Cupcake theme:

SCR-20230602-jfls

Maybe we can just sort the badges so that the language the most used is displayed first

This was already the case as I added this directly in the GraphQL query through the orderBy parameter (orderBy: { direction: DESC, field: SIZE }).

I'm even wondering if we shouldn't remove the languages under xx% but I'm not sure about it.

This should be a fairly easy addition if needed as we would basically just need a top-level constant to define a threshold (either in bytes or a percentage). We are already filtering out the Shell language code below 2000 bytes as it's a common false positive due to pre-commit hooks.


As we don't need anymore the colors and the percentages right now, this PR has been refactored to no longer store these informations and it also greatly reduces the code / tests as the percentage calculation was done manually considering that GitHub only returns individual sizes in bytes.

If we ever need to bring back colors or percentages, e.g. if we want to use percentage as a threshold to filter out languages, we should be able to refer to the previous implementation in this pull request to restore it.

@HiDeoo HiDeoo marked this pull request as ready for review June 2, 2023 08:04
@julien-deramond julien-deramond self-requested a review June 2, 2023 08:08
@julien-deramond julien-deramond added the feature Something can be improved label Jun 2, 2023
Copy link
Member

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

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

We're on the same page regarding your last comment :shipit:
Thanks a lot for this PR, nicely executed and detailed! 🔥🔥🔥

@julien-deramond julien-deramond merged commit 19e6db9 into Open-reSource:main Jun 2, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Something can be improved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants