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
Update transaction details drawer #10374
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @gustavlrsn and the rest of your teammates on Graphite |
ad159a4
to
3c66bbe
Compare
3c66bbe
to
4f4d7dd
Compare
4f4d7dd
to
a590089
Compare
a590089
to
9fb916d
Compare
9fb916d
to
641895c
Compare
f2f59dc
to
995c8f6
Compare
995c8f6
to
fa5c8c2
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.
Looking and feeling good. Nice work!
onClickRow={(row, menuRef) => openDrawer(row.id, menuRef)} | ||
onClickRow={(row, menuRef) => { | ||
openDrawer(row.id, menuRef); | ||
}} | ||
onHoverRow={row => setHoveredGroup(row?.original?.group ?? null)} |
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 caught something while testing the performance of the Transactions dashboard: onHoverRow
causes the whole table to re-render when hovering over a different group. This is fine for now because we do not display a lot of transactions in the table right now, but it could become an issue if we decide to increase the number of rows being displayed at the same time. A possible solution would be implementing this effect using CSS selectors and data properties.
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.
Good point, created an issue to track this: opencollective/opencollective#7405
components/CopyId.tsx
Outdated
export const CopyID = ({ children, tooltipLabel = <FormattedMessage defaultMessage="Copy ID" id="wtLjP6" /> }) => { | ||
export const CopyID = ({ | ||
children, | ||
prepend = '', |
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 probably be more straightforward to inform a value
property you want to be copied and always display children as the label.
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.
Good suggestion, pushed a fix for this!
fa5c8c2
to
edcfb06
Compare
edcfb06
to
176f3a4
Compare
176f3a4
to
456ba44
Compare
Resolve opencollective/opencollective#7396
Project: opencollective/opencollective#7343
Description
Changes to the Transaction details drawer
Updates the payment method label to be a combination of service and type
Screenshots