-
Notifications
You must be signed in to change notification settings - Fork 3.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
chore(dashboard): side nav updates #6658
Conversation
@@ -59,7 +60,7 @@ export const FreeTrialCard = () => { | |||
Experience novu without any limits for free for the next {pluralizedDays}. | |||
</span> | |||
<div className={`max-h-3 overflow-hidden opacity-100 ${transition} group-hover:max-h-0 group-hover:opacity-0`}> | |||
<Progress value={daysLeft} max={subscription.trial.daysTotal > 0 ? subscription.trial.daysTotal : 100} /> | |||
<Progress value={daysTotal - daysLeft} max={daysTotal} /> |
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.
fixed the issue with the wrong progress value for the free trial card
organizationSwitcherTriggerIcon: 'ml-auto', | ||
'group w-full py-0 [&>.cl-organizationPreview]:px-0 px-1.5 justify-start hover:bg-background before:border-neutral-alpha-100 focus-visible:ring-ring group relative flex cursor-pointer items-center gap-2 rounded-lg transition duration-300 ease-out before:absolute before:bottom-[1px] before:left-0 before:h-0 before:w-full before:border-b before:border-solid before:transition-all before:duration-300 before:ease-out before:content-[""] hover:shadow-sm hover:before:border-transparent focus-visible:outline-none focus-visible:ring-1 focus:bg-transparent focus:shadow-sm focus:bg-background focus:before:border-transparent', | ||
organizationSwitcherTriggerIcon: | ||
'opacity-0 ml-auto transition duration-300 ease-out group-hover:opacity-100 group-focus:opacity-100', |
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.
by default we want to hide the dropdown trigger icon, but show it on hover/focus
if (Modal) { | ||
return ( | ||
<Modal> | ||
<span className={classNames}>{children}</span> | ||
</Modal> | ||
); | ||
} |
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.
added ability to render modal when clicking on the nav item
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'm not sure I like this abstraction. Can't we just use asChild
and render the button that opens the corresponding modal when necessary?
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 can, but we want to render fully custom modals for each link. They might differ in header, content, and footer docs link. If you know how to do this better, please suggest.
import { Button } from '../primitives/button'; | ||
import { RiBookMarkedLine } from 'react-icons/ri'; | ||
|
||
export const SubscribersStayTunedModal = ({ children }: { children: ReactNode }) => { |
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.
when the subscribers side nav link is clicked this modal will be shown
✅ Deploy Preview for novu-stg-vite-dashboard-poc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
if (Modal) { | ||
return ( | ||
<Modal> | ||
<span className={classNames}>{children}</span> | ||
</Modal> | ||
); | ||
} |
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'm not sure I like this abstraction. Can't we just use asChild
and render the button that opens the corresponding modal when necessary?
novu
@novu/headless
@novu/framework
@novu/client
@novu/nest
@novu/node
@novu/js
@novu/notification-center
@novu/providers
@novu/react
@novu/react-native
@novu/shared
@novu/stateless
commit: |
<DialogPrimitive.Overlay | ||
ref={ref} | ||
className={cn( | ||
'data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 fixed inset-0 z-50 bg-neutral-950/10', |
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.
bg-neutral-100
?
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 doesn't have alpha, but even the alpha version is not the same as what is in the Figma, which is hsla(222, 32%, 8%, 0.1)
but this bg-neutral-950/10
doesn't produce the same result but really close :(
What changed? Why was the change needed?
Updated the Dashboard Side Navigation component with the latest design changes.
Screenshots
Screen.Recording.2024-10-09.at.09.59.51.mov