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
[features][ML listview] Sort on table headers #14963
[features][ML listview] Sort on table headers #14963
Conversation
…trapi/strapi into ML-listview/sort
Codecov ReportBase: 49.78% // Head: 59.77% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## features/MediaLibrary-listview #14963 +/- ##
==================================================================
+ Coverage 49.78% 59.77% +9.98%
==================================================================
Files 290 1343 +1053
Lines 10189 32712 +22523
Branches 2254 6239 +3985
==================================================================
+ Hits 5073 19554 +14481
- Misses 4217 11298 +7081
- Partials 899 1860 +961
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
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 works well, I just have one idea
<IconButton | ||
label={sortLabel} | ||
onClick={() => handleClickSort(isSorted, name)} | ||
icon={isUp ? <CarretUp /> : <CarretDown />} |
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.
Worth passing a children instead? 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.
why not yes! just out of curiosity, is there a noticeable difference between passing it as prop/children? for better reading?
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.
LGTM 👍
What
Make
name
,createdAt
andupdatedAt
columns sortableTests
Sort on any of those attributes
Should be accessible through keyboard + voice reader tools