Skip to content
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

fix(data-table): return sortDirection in onSortChange #1301

Merged
merged 3 commits into from
Mar 26, 2020

Conversation

jonnybel
Copy link
Contributor

@jonnybel jonnybel commented Mar 24, 2020

Summary

This PR adjusts the DataTable to have the onSortChange fn use the next sortDirection in its interface.

Description

This is useful for situations (which already exist in the MC) where the sorting isn't done in the client-side with the useSorting hook, but is instead performed by the server and we're letting the table internally control and provide the sortDirection to the consumer.

I'm marking this as a fix because the old table already provides this interface, and while I initially thought it unnecessary, I found it useful for easing the migration from the old table to the new one.

@jonnybel jonnybel added the 🐛 Type: Bug Something isn't working label Mar 24, 2020
@jonnybel jonnybel self-assigned this Mar 24, 2020
@vercel
Copy link

vercel bot commented Mar 24, 2020

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/commercetools/ui-kit/6pafsl7ce
✅ Preview: https://ui-kit-git-fix-data-table-on-sort.commercetools.now.sh

Copy link
Contributor

@tdeekens tdeekens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dang it. Didn't submit my two day old review. Sorry.

@kodiakhq kodiakhq bot merged commit cc0ab54 into master Mar 26, 2020
@kodiakhq kodiakhq bot deleted the fix-data-table-on-sort branch March 26, 2020 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants