feat: add new label to swaps button [SWAP-86] #3763
Conversation
Branch preview✅ Deploy successful! Storybook: |
| @@ -12,7 +12,7 @@ const darkPalette = { | |||
| secondary: { | |||
| dark: '#636669', | |||
| main: '#FFFFFF', | |||
| light: '#12FF80', | |||
| light: '#B0FFC9', | |||
There was a problem hiding this comment.
This was synced with @TanyaEfremova
ESLint Summary View Full Report
Report generated by eslint-plus-action |
| if (item.href === AppRoutes.swap) { | ||
| return <Chip label="New" color="success" sx={{ ml: 1 }} /> | ||
| } |
There was a problem hiding this comment.
Should this be behind a feature flag or be a configuration constant somewhere? I don't know when we want to ermove it, but then we'll need to go in this file an delete this code.
📦 Next.js Bundle Analysis for safe-wallet-webThis analysis was generated by the Next.js Bundle Analysis action. 🤖
|
| Page | Size (compressed) |
|---|---|
global |
949.98 KB (🟡 +54 B) |
Details
The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.
Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis
If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!
Coverage report
Show files with reduced coverage 🔻
Test suite run success1436 tests passing in 197 suites. Report generated by 🧪jest coverage report action from a2de336 |
| return <SidebarListItemCounter count={queueSize} /> | ||
| } | ||
|
|
||
| if (item.href === AppRoutes.swap) { |
There was a problem hiding this comment.
Since this is just a static property, I would move it to the navItems config. The queue is in this dynamic code only because we need to count the queue size.
There was a problem hiding this comment.
good idea! Please check again.
| } | ||
|
|
||
| if (item.href === AppRoutes.swap) { | ||
| return <Chip label="New" color="success" sx={{ ml: 1 }} /> |
There was a problem hiding this comment.
@TanyaEfremova what chip style should be used here? We've used this one in other places:
There was a problem hiding this comment.
Yes, I initially went for the "new chip" that we have, but that is not what is in the figma design.
There was a problem hiding this comment.
I would use the new chip for the navigation only. Also see the new chip used for pending transactions counter. The one on the screenshot would be too big for the navigation, so it will be re-used.
We can definitely merge the 'New' chip and use it for the newly released features
compojoom
force-pushed
the
feat-add-new-badge
branch
from
25d775a
to
6548aa3
Compare
| @@ -98,7 +101,7 @@ const Navigation = (): ReactElement => { | |||
| <SidebarListItemText data-testid="sidebar-list-item" bold> | |||
| {item.label} | |||
|
|
|||
| <SidebarListItemCounter count={getCounter(item)} /> | |||
| <NavItemTag item={item} /> | |||
There was a problem hiding this comment.
You could pass queue size here to avoid calling the useQueuedTxsLength for each item.
There was a problem hiding this comment.
ok, i've refactored it.
| @@ -31,6 +33,7 @@ export const navItems: NavItem[] = [ | |||
| label: 'Swap', | |||
| icon: <SvgIcon component={SwapIcon} inheritViewBox />, | |||
| href: AppRoutes.swap, | |||
| tag: <Chip label="New" color="success" sx={{ ml: 1 }} />, | |||
There was a problem hiding this comment.
Looks good! Alternatively it could be a boolean prop like isNew. Both are fine.
There was a problem hiding this comment.
No, I like it being a react component. This way we could add whatever we want to other items
What it solves
Displays a "new" label next to the swap navitem
How to test it
Open a safe and in the menu on the right Swap should have a "new" badge/label.
Screenshots
Checklist