onSort with *correct* sortProperties #749
Conversation
| })) | ||
| )(props => <OriginalComponent {...props} />); | ||
| // Obsolete | ||
| const EnhancedHeadingCell = OriginalComponent => OriginalComponent; |
There was a problem hiding this comment.
It can, but it's technically a breaking change. Glad to remove if you're not concerned.
There was a problem hiding this comment.
Just to double confirm - we're leaving this here to avoid causing a potential breaking change right? I think we're maybe just outside the realm of where a 2.0 release makes sense. If I'm understanding correctly, we can leave this here for now to avoid breaking change and remove the obsolete field in a 2.0 version.
Thoughts?
There was a problem hiding this comment.
Correct. I left it as a no-op to be cleaned up for a 2.0 release.
The chances someone is actually depending on this seem quite low, but it's been exported as part of the public API.
|
On re-review I noticed a few gaps between core and local versions of |
Griddle major version
1.x
Changes proposed in this pull request
onSortis now fired just before the newsortPropertiesare dispatched.This essentially renders
TableHeadingCellobsolete, but deleting it is a potentially breaking change.Idea
What if events were fired from a Redux middleware instead of click handlers?
Why these changes are made
Close #716; #717 was insufficient.
Are there tests?
Existing story now demonstrates expected behavior.
It seems like a new React version changed behavior when
render()returns a string, so the TS story has been adjusted.