fix(text-field): Made handleValidationAttributeMutation to accept attribute list #2794
Conversation
|
Please fix lint issues |
packages/mdc-textfield/foundation.js
Outdated
| * @param {!Array<!MutationRecord>} mutationsList | ||
| * @return {!Array<string>} Returns list of attribute names. | ||
| */ | ||
| getMutationAttributeNames_(mutationsList) { |
There was a problem hiding this comment.
This should be an adapter method. We should be removing the MutationRecord from the foundation, since its environment specific.
There was a problem hiding this comment.
This method is no longer required here since this part is moved to index.js (registerValidationAttributeChangeHandler's implementation) to separate the concerns of MutationRecord logic. PTAL.
|
Discussed this with Abhinay, and we realized we should be able to solve this with a 1-2 line change in the vanilla DOM adapter implementation for The adapter will be changed to expect the handler to handle an array of strings rather than an array of mutation objects, so it'll require a README update and a BREAKING CHANGE note in the commit description. The foundation method will also change to require an array of strings, which also needs a doc update but isn't a breaking change itself since we only just made the method public after the last release. (Should we change the name to This way, no additional adapter methods need to be implemented. In MDC React, the logic should change to look at |
…geHandler for handler. handler in registerValidationAttributeChangeHandler now accepts the list of attribute changes instead of mutations for isolating the MutationRecord off from foundation.
Codecov Report
@@ Coverage Diff @@
## master #2794 +/- ##
==========================================
+ Coverage 98.48% 98.48% +<.01%
==========================================
Files 98 98
Lines 4231 4232 +1
Branches 538 538
==========================================
+ Hits 4167 4168 +1
Misses 64 64
Continue to review full report at Codecov.
|
…ionrecord_ref_issue2717
…andleValidationAttributeChange. And updated tests.
|
The function name change sounds good to me. w/ regards to the React, if we don't have the list of attributes to look for, we will be calling the handleValidationChange every time a prop changes. |
|
Hm. If we move MDC Web's whitelist into constants.js, can MDC React reference it there rather than needing to redefine it? |
|
Ya that's a good idea 👍 |
packages/mdc-textfield/adapter.js
Outdated
| @@ -102,7 +102,8 @@ class MDCTextFieldAdapter { | |||
|
|
|||
| /** | |||
| * Registers a validation attribute change listener on the input element. | |||
| * @param {function(!Array): undefined} handler | |||
| * Handler accepts list of attribute changes. | |||
| @@ -319,7 +319,7 @@ test('#adapter.deregisterTextFieldInteractionHandler removes an event handler fo | |||
| test('#adapter.registerValidationAttributeChangeHandler creates a working mutation observer', (done) => { | |||
| const {root, component} = setupTest(); | |||
| const handler = td.func('ValidationAttributeChangeHandler'); | |||
| td.when(handler(td.matchers.anything(), td.matchers.anything())).thenDo(() => { | |||
| td.when(handler(td.matchers.anything())).thenDo(() => { | |||
There was a problem hiding this comment.
anything -> contains('required')?
…ionrecord_ref_issue2717
…ribute list (#2794) BREAKING CHANGE: registerValidationAttributeChangeHandler adapter API now expects the handler to accept an array of strings, not mutation objects
Fixes #2717
BREAKING CHANGE:
registerValidationAttributeChangeHandler.handleValidationAttributeMutation=>handleValidationAttributeChange.