-
Notifications
You must be signed in to change notification settings - Fork 39
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: new features highlight & wallet landing #1166
Conversation
🦋 Changeset(s) detectedThis PR includes changeset(s) for the following changed packages:
Not sure what this means? Click here to learn what changesets are. |
806c784
to
708a3d4
Compare
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.
looks great, just few minor comments
apps/extension/src/ui/apps/popup/pages/Portfolio/PortfolioAccounts.tsx
Outdated
Show resolved
Hide resolved
apps/extension/src/ui/apps/popup/pages/Portfolio/PortfolioAccounts.tsx
Outdated
Show resolved
Hide resolved
e085d50
to
0ea59d3
Compare
<HandMonoLogo className="text-base" /> | ||
Talisman | ||
</div> | ||
<div className="text-primary text-2xl font-extrabold">V{process.env.VERSION}</div> |
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.
I don't think this should always show the latest version, but the version associated with the features listed. For example, if we release some major features in 1.X.0, it would be a bit confusing if this said 1.X.1, suggesting that the new features came out in the minor release.
To deal with this we could either code a mapping between "what's new message" and release version, or just go back to the idea of including the version number as part of the image rather than a separate html overlay.
*/ | ||
export const WhatsNewVersion = 1 | ||
|
||
export const PortfolioWhatsNew = () => { |
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.
It would be great if we could make this component generic, and just have it accept a JSON of values configuring the What's New message according to some standard format. This way anyone (@0byt for example) could submit a PR with a JSON object for a new message.
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.
Some changes to the What's New setup requested
1d502e5
to
67e3cd0
Compare
67e3cd0
to
e85fa0b
Compare
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.
LGTM!
No description provided.