-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Worked on the Usage column & Lineage Drawer #6290
Worked on the Usage column & Lineage Drawer #6290
Conversation
@@ -161,6 +161,7 @@ export default function SchemaTable({ | |||
dataIndex: 'fieldPath', | |||
key: 'usage', | |||
render: usageStatsRenderer, | |||
sorter: (sourceA, sourceB) => sourceA.name.localeCompare(sourceB.name), |
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 believe we would want to sort this by the count of the usage column and not the name. I'm actually not even sure what name
would be in this situation here?
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.
yeah when I try to use this code to sort, I get a runtime error and react breaks. were you able to find any data locally with usage to test this on?
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 believe you're going to have to write a function that takes into account usageStats
above and finds the fields you're sorting and then sorts by count
that you find by field in usageStats
above.
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 looks like your sorting functions aren't actually sorting based on what we would expect for the columns.. at least one of them also breaks the frontend when trying to sort
@@ -192,6 +192,7 @@ export const AccessTokens = () => { | |||
<span>{`${formattedExpireAt.toLocaleDateString()} at ${formattedExpireAt.toLocaleTimeString()} (${localeTimezone})`}</span> | |||
); | |||
}, | |||
sorter: (sourceA, sourceB) => sourceA.name.localeCompare(sourceB.name), |
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.
same questions here... why are you sorting by name
for the "Expires At" column? shouldn't we be sorting based on that date for this column?
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.
nice stuff!
Checklist