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: use new design for FileContributors #14898

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

JoeChenJ
Copy link
Contributor

Description

Use new design for FileContributors

Related Issue

#8024

@github-actions github-actions bot added content 🖋️ This involves copy additions or edits tooling 🔧 Changes related to tooling of the project translation 🌍 This is related to our Translation Program labels Feb 13, 2025
Copy link

netlify bot commented Feb 13, 2025

Deploy Preview for ethereumorg ready!

Name Link
🔨 Latest commit e39dfe8
🔍 Latest deploy log https://app.netlify.com/sites/ethereumorg/deploys/67af0d9cc98e2f00081e3c3d
😎 Deploy Preview https://deploy-preview-14898--ethereumorg.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
7 paths audited
Performance: 46 (🔴 down 5 from production)
Accessibility: 95 (no change from production)
Best Practices: 89 (🔴 down 9 from production)
SEO: 98 (no change from production)
PWA: 59 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@JoeChenJ
Copy link
Contributor Author

Hey @konopkja, could you take a look at this?

Let me know if anything needs improvement!

@konopkja
Copy link
Contributor

Screenshot 2025-02-14 at 9 09 23

looks awesome!

I think this gap could be 50% of what it is now, but overall its great!


I noticed that the component is only visible on desktop, do you intend on preparing it for mobile also? I think there is enough space

@JoeChenJ
Copy link
Contributor Author

Screenshot 2025-02-14 at 9 09 23

looks awesome!

I think this gap could be 50% of what it is now, but overall its great!

I noticed that the component is only visible on desktop, do you intend on preparing it for mobile also? I think there is enough space

@konopkja Sure, I'll work on this soon!

@JoeChenJ
Copy link
Contributor Author

Hey @konopkja , I’ve adjusted the gap and made the updates for mobile. I also want to point out a few additional changes I made that you might not notice:

  1. I added a “Translator” tag next to the Crowdin contributors.

  2. I changed the redirect logic for Crowdin contributors — they now link to their Crowdin profiles instead of GitHub.

  3. On pages with Crowdin contributors, the modal now displays both Crowdin and GitHub contributors (it used to show only one or the other).

Could you take a look at those as well and let me know if everything looks fine? Appreciate your feedback!

@konopkja
Copy link
Contributor

Hey @konopkja , I’ve adjusted the gap and made the updates for mobile. I also want to point out a few additional changes I made that you might not notice:

  1. I added a “Translator” tag next to the Crowdin contributors.
  2. I changed the redirect logic for Crowdin contributors — they now link to their Crowdin profiles instead of GitHub.
  3. On pages with Crowdin contributors, the modal now displays both Crowdin and GitHub contributors (it used to show only one or the other).

Could you take a look at those as well and let me know if everything looks fine? Appreciate your feedback!

Hey @JoeChenJ it looks perfect! Really appreciate this particular contribution <3

@konopkja
Copy link
Contributor

shall we discuss extending this component to other pages outside developer docs or would it be better tackled in another PR?

@JoeChenJ
Copy link
Contributor Author

shall we discuss extending this component to other pages outside developer docs or would it be better tackled in another PR?

I think it would be best to merge this PR first, and then we can extend it in another one, as it might take a bit more time

@JoeChenJ JoeChenJ marked this pull request as ready for review February 14, 2025 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content 🖋️ This involves copy additions or edits tooling 🔧 Changes related to tooling of the project translation 🌍 This is related to our Translation Program
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants