[fix:dropdown, grid, modal] WB-554: update components for use with react-hot-loader #390
Conversation
kevinbarabash
requested review from
jeresig and
somewhatabstract
as code owners
|
Deploy preview for wonder-blocks ready! Built with commit eb5e806 |
Codecov Report
@@ Coverage Diff @@
## master #390 +/- ##
==========================================
+ Coverage 94.3% 94.32% +0.02%
==========================================
Files 107 107
Lines 1492 1499 +7
Branches 288 295 +7
==========================================
+ Hits 1407 1414 +7
Misses 79 79
Partials 6 6
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #390 +/- ##
==========================================
+ Coverage 94.3% 94.32% +0.02%
==========================================
Files 107 107
Lines 1492 1499 +7
Branches 288 295 +7
==========================================
+ Hits 1407 1414 +7
Misses 79 79
Partials 6 6
Continue to review full report at Codecov.
|
I wonder if they plan to or if this is a WON'T FIX. |
|
I found this and added a comment (mainly to revive it a bit - it hasn't been commented on since 2017 until today). |
|
Are these changes going to result in a new patch version for each component (e.g. v1.1.0 -> v1.1.1)? If that's the case, is that a manual change? or do we have a script that does that? |
|
@jandrade running |
kevinbarabash
changed the title
|
I should've changed the title before merging, d'oh. |
## Summary: This PR add the new props `beforeNav` and `safeWithNav` to `Button` and `ClickableBehavior`. These props were proposed in [ADR #390](https://docs.google.com/document/d/1p4UUOHEqhaXG7rVg7J4j6S0a83wF76QcPqnEJvrtqyA/edit) with one minor change, they must always return a promise. This provides a clear separation of responsibility: synchronous actions should be handled by `onClick` (which can be used with or without an `href`) and asynchronous actions should be handled by `beforeNav` or `safeWithNav` depending on the desired behavior. Since this PR modifies `ClickableBehavior` a number of other components required minor changes in order for flow type-checking to succeed. Future PRs will finish updating these components to have `beforeNav` and `safeWithNav` props just like `Button`. This PR also updates the build configuration so that async functions aren't transformed. TODO: - [ ] ~touch events~ this is already handled since we're relying on the mobile browser to synthesize a "click" event for us. - [x] keyboard events - [x] dedupe logic between mouse and keyboard event handling of `beforeNav` and `safeWithNav` - [x] documentation - [x] add tests for `beforeNav` and `safeWithNav` to clickable-behavior.test.js - [x] add tests for `spinner` behavior Issue: WEB-2990 ## Test plan: - yarn jest packages/wonder-blocks-button/components/__tests__/button.test.js Reviewers: #fe-infra Author: kevinbarabash Reviewers: aag, kevinbarabash, jeresig Required Reviewers: Approved by: jeresig, jeresig Checks: ✅ Mixed content, ❓ Redirect rules, ❓ Header rules, ⏭ Auto-approve dependabot PRs, ❓ Pages changed, ✅ chromatic Pull request URL: #1047
The issue is that react-hot-loader wraps components in ReactProxy classes which breaks direct component class checks.
Test Plan:
Aside: I really wanted to use
Symbolso I could copy/paste theisClassOfsnippet without having to modify it each time, but flow doesn't support computed static vars. 😞