-
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
feat(dashboard): Make the whole workflow row clickable #7408
feat(dashboard): Make the whole workflow row clickable #7408
Conversation
✅ Deploy Preview for dev-web-novu ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for dashboard-v2-novu-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -160,47 +180,43 @@ export const WorkflowRow = ({ workflow }: WorkflowRowProps) => { | |||
}; | |||
|
|||
return ( | |||
<TableRow key={workflow._id} className="relative"> | |||
<TableRow key={workflow._id} className="group relative isolate"> |
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.
What if we just do:
<TableRow key={workflow._id} className="hover:bg-accent/50 relative hover:cursor-pointer" onClick={handleRowClick}>
On the main row, and add to the onHoverCopy and actions cell:
onClick={(e) => e.stopPropagation()}
Might simplify this a bit, wdyt?
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 implementation feels like an overkill, but if I am not mistaken, it's the only way to maintain valid Table/tr/td HTML while treating each row as a link, making right-clicking Open in a new Tab work.
@desiprisg Please confirm.
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.
Yes that's the thinking behind this. Also checked with some other implementations like stripe's and they seem to follow the same approach.
What changed? Why was the change needed?
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer