-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Add 'Ethereum Ecosystem' CTA to ethereum.org #12373
Conversation
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
This is too aggressive ad, it is above the list of curated dapps on the page itself. It is a worthy addition for users who seek an expanded list of dapps or to see the whole ecosystem, but will not be useful to a beginner who needs less options and more curation/guidance, which https://www.ethereum-ecosystem.com/ currently does not offer. I suggest to rethink the position and how to link to this project on this page. |
Hi @konopkja , Thanks for the constructive feedback on the CTA. I see your point about its prominence possibly being overwhelming for newcomers. Inspired by the L2Beat CTA on ethereum.org's Layer 2 overview, I was aiming for similar visibility for Ethereum Ecosystem. Would you be open to placing the CTA lower on the page, where it might be more contextually appropriate for users looking for expanded information? I'm keen to adjust the approach to better fit the community's needs. Looking forward to your guidance. |
WalkthroughThe update involves adding a new feature to the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
@konopkja what do you think about this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
src/intl/en/page-dapps.json
is excluded by:!**/*.json
Files selected for processing (1)
- src/pages/dapps.tsx (2 hunks)
Additional comments: 1
src/pages/dapps.tsx (1)
- 125-125: The import statement for the new image
exploring
is correctly added and follows the existing pattern for importing images in this file. This ensures consistency in how resources are managed within the project.
src/pages/dapps.tsx
Outdated
{/* Explore Ecosystem Section */} | ||
<Content> | ||
<CenterDivider /> | ||
<CalloutBanner | ||
mt={0} | ||
mx={0} | ||
mb={{ base: 0, lg: 16 }} | ||
titleKey={"page-dapps:page-dapps-explore-title"} | ||
descriptionKey={"page-dapps:page-dapps-explore-1"} | ||
image={exploring} | ||
imageWidth={300} | ||
alt={t("page-dapps-explore-callout-image-alt")} | ||
> | ||
<Box> | ||
<ButtonLink to="https://www.ethereum-ecosystem.com/apps"> | ||
{t("page-dapps:page-dapps-explore-2")} | ||
</ButtonLink> | ||
</Box> | ||
</CalloutBanner> | ||
</Content> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The addition of the "Explore Ecosystem" section using a CalloutBanner
component is well-integrated into the page. The ButtonLink
correctly directs users to the "Ethereum Ecosystem" website. However, it's important to ensure that the URL (https://www.ethereum-ecosystem.com/apps
) is stable and maintained, as it directs users to an external site. Additionally, consider adding rel="noopener noreferrer"
to the ButtonLink
for security reasons when opening links in a new tab.
- <ButtonLink to="https://www.ethereum-ecosystem.com/apps">
+ <ButtonLink to="https://www.ethereum-ecosystem.com/apps" rel="noopener noreferrer">
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
{/* Explore Ecosystem Section */} | |
<Content> | |
<CenterDivider /> | |
<CalloutBanner | |
mt={0} | |
mx={0} | |
mb={{ base: 0, lg: 16 }} | |
titleKey={"page-dapps:page-dapps-explore-title"} | |
descriptionKey={"page-dapps:page-dapps-explore-1"} | |
image={exploring} | |
imageWidth={300} | |
alt={t("page-dapps-explore-callout-image-alt")} | |
> | |
<Box> | |
<ButtonLink to="https://www.ethereum-ecosystem.com/apps"> | |
{t("page-dapps:page-dapps-explore-2")} | |
</ButtonLink> | |
</Box> | |
</CalloutBanner> | |
</Content> | |
{/* Explore Ecosystem Section */} | |
<Content> | |
<CenterDivider /> | |
<CalloutBanner | |
mt={0} | |
mx={0} | |
mb={{ base: 0, lg: 16 }} | |
titleKey={"page-dapps:page-dapps-explore-title"} | |
descriptionKey={"page-dapps:page-dapps-explore-1"} | |
image={exploring} | |
imageWidth={300} | |
alt={t("page-dapps-explore-callout-image-alt")} | |
> | |
<Box> | |
<ButtonLink to="https://www.ethereum-ecosystem.com/apps" rel="noopener noreferrer"> | |
{t("page-dapps:page-dapps-explore-2")} | |
</ButtonLink> | |
</Box> | |
</CalloutBanner> | |
</Content> |
its better, though this is a very big recognition - using a full width banner like this, there are only few instances when we have done that before and those were open source communities not a closed project so I am still hesitant on this approach. For now lets keep this open and see what others say. |
agree with @wackerow and i would suggest changing it to a text based link perhaps |
…t this is a third party website
src/pages/dapps.tsx
Outdated
<ButtonLink | ||
to="https://www.ethereum-ecosystem.com/apps" | ||
title={t("page-dapps:page-dapps-explore-1")} | ||
> | ||
{t("page-dapps:page-dapps-explore-title")} | ||
</ButtonLink> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@konopkja and I were both suggesting a text link here... but will ultimately defer to him on this.
As for the usage of title
, I'd say this is a very subtle way to let people know, while I was imagining something much more obvious and attention grabbing where that notice cannot be missed. @konopkja ideas?
Maybe:
Want to browse more? [Check out thousands of dapps on the Ethereum Ecosystem\* website](https://www.ethereum-ecosystem.com/apps)
_(\*Ethereum Ecosystem is not affiliated with ethereum.org or the Ethereum Foundation)_
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @wackerow,
Sorry I misunderstood the text link suggestion earlier.
I've spent about an hour attempting to configure it in a way that ensures the warning stands out next to the text link, but I must admit it's proving quite challenging to balance the aesthetics with the practicality of the warning notice.
@konopkja @wackerow What do you think of this?
Thank you for your understanding and patience so far as we strive to create a solution that is user-friendly and also clearly communicates that Ethereum Ecosystem is independent of Ethereum.org.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ping @konopkja... I would personally keep the link on the same line as the text; we use the pattern a lot and typically don't line-break. The warning, however, I think looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- yeah i think this needs some styling to do to look appropriate on the page
1A. "Want to browse more?" should be bold and change to "Want to browse more apps?", 1B. the link phrase is too long - remove "on Ethereum Ecosystem's website" - the disclaimer isnt necessary, we dont provide it for other external links either so remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"page-dapps-artblocks-image-alt": "Art Blocks logo" | ||
"page-dapps-artblocks-image-alt": "Art Blocks logo", | ||
"page-dapps-explore-title": "Want to browse more apps?", | ||
"page-dapps-explore": "Check out hundreds of dapps" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"page-dapps-explore": "Check out hundreds of dapps" | |
"page-dapps-explore": "Check out more dapps" |
Dont have a strong opinion here, but think this would be better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine either way here... "hundreds" does kinda drive home the point that there are a lot of them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Maxservais!
In general I think this is ready to go. Going to bring this up next GitHub grooming session to see if there are any blockers/reservations left with what is here, and if not get this wrapped up.
This PR introduces a Call to Action on https://ethereum.org/en/dapps/, guiding visitors to Ethereum Ecosystem, an unofficial ecosystem page for Ethereum and its L2s.
Description
This addition aims to provide enhanced visibility for a resource that catalogues over 1000 dApps across Ethereum and its Layer 2s, facilitating greater user exploration and engagement within the ecosystem.
Related Issue
Preview URL
Summary by CodeRabbit