-
Notifications
You must be signed in to change notification settings - Fork 2
remove mentions of vision and format #709
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
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
5 Skipped Deployments
|
lightwalker-eth
left a comment
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.
@notrab Appreciate these updates. Shared a few suggestions 👍 I can confirm that the unit tests that are breaking are unrelated to the changes in this PR and will need to be investigated as a separate effort some other time.
apps/namegraph.dev/lib/utils.ts
Outdated
| export const ExternalLinkHosts = { | ||
| "Vision": "Vision", | ||
| "ENSDomains": "ENSDomains" | ||
| Vision: "Vision", |
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.
This caught my eye
apps/namegraph.dev/lib/utils.ts
Outdated
| case ExternalLinkHosts.ENSDomains: | ||
| return `https://app.ens.domains/${name}` | ||
| return `https://app.ens.domains/${name}`; | ||
| case ExternalLinkHosts.Vision: |
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.
Appreciate your review here
| case ExternalLinkHosts.ENSDomains: | ||
| return <EnsOutlineIcon className="w-8 h-8 hover:scale-110 transition" />; | ||
| case ExternalLinkHosts.Vision: | ||
| return <EnsVisionIcon className="w-16 h-16 hover:scale-110 transition" />; |
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.
Please:
- Also remove this
EnsVisionIconcomponent. - Search all our files (both file names and file contents) for "vision" or "Vision" or etc.. to find anything else we missed.
| switch (host) { | ||
| case ExternalLinkHosts.ENSDomains: | ||
| return <EnsOutlineIcon className="w-8 h-8 hover:scale-110 transition" />; | ||
| case ExternalLinkHosts.Vision: |
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.
We want to replace the link to Vision with a link (and icon) for OpenSea instead.
lightwalker-eth
left a comment
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.
@notrab Thanks for your help with this 👍
vision(example implementation banners).formatwhich hasn't been done in a while on this repo.