feat: support @testing-library/react-native 7.0 #50

brunohkbx · 2020-11-12T15:42:10Z

Rewrite toBeDisabled asserting the logic RN does around onStartShouldSetResponder as discussed in #23

Documentation added to the
docs
Typescript definitions updated
Tests
Ready to be merged

Resolves #43

codecov · 2020-11-12T17:00:51Z

Codecov Report

Merging #50 (f63a819) into master (3cfd279) will decrease coverage by 3.65%.
The diff coverage is 100.00%.

❗ Current head f63a819 differs from pull request most recent head 1f1bda8. Consider uploading reports for the commit 1f1bda8 to get more accurate results

@@             Coverage Diff             @@
##            master      #50      +/-   ##
===========================================
- Coverage   100.00%   96.34%   -3.66%     
===========================================
  Files            8        8              
  Lines           96       82      -14     
  Branches        28       26       -2     
===========================================
- Hits            96       79      -17     
- Misses           0        1       +1     
- Partials         0        2       +2

Impacted Files	Coverage Δ
src/to-be-disabled.js	`60.00% <100.00%> (-40.00%)`	⬇️
src/utils.js	`93.75% <0.00%> (-6.25%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3cfd279...1f1bda8. Read the comment docs.

thymikee · 2020-11-12T17:20:14Z

Um, I don't see why would we remove the feature to add it once again? Maybe we could extract some helpers from RNTL that could be used by toBeDisabled? I'm in favor of removing toBeEnabled though, as it should be inverse of disabled.

brunohkbx · 2020-11-17T20:44:03Z

@thymikee sorry for taking too long to answer.
I thought exactly the same. I'll try to reimplement toBeDisabled in this PR

deriegle · 2020-12-18T21:22:30Z

I'll try to reimplement toBeDisabled in this PR

Is there still work being done on this? We'd really like to be able to use toBeDisabled in our codebase again, but it doesn't work as expected after upgrading to v7 of RNTL.

brunohkbx · 2021-04-06T19:49:11Z

@thymikee @mdjastrzebski after some months I'm finally back to this PR. sorry for that.

I have some questions:

Do you guys think this PR should be a breaking change? Or is it fine to release in the current 4.0?
Looks like RNTL master branch is quite different than this PR #460 from @mdjastrzebski. Should we still rely only on onStartShouldSetResponder? Or we could also check pointerEvents and accessibilityState?

mdjastrzebski

I've added some thoughts about the tests and vague idea about the implementation, these might be relevant but might be not because I haven't worked with RNTL internals lately.

mdjastrzebski · 2021-04-08T20:38:44Z

src/to-be-disabled.js

  checkReactElement(element, toBeDisabled, this);

-  const isDisabled = isElementDisabled(element) || isAncestorDisabled(element);
+  const isDisabled = !(element.props?.onStartShouldSetResponder?.() ?? true);


In my opinion we should mimic the current RNTL 7.2 logic here: isEventEnabled function & it's deps. Potentially the check might have to be recursive as fireEvent implementation in RNTL is performing recursive checks.

mdjastrzebski · 2021-04-08T20:43:02Z

src/__tests__/to-be-disabled.js

+  test.each([
+    ['Button', Button, { title: 'some button' }],
+    ['TouchableOpacity', TouchableOpacity, {}],
+    ['TouchableHighlight', TouchableHighlight, {}],
+    ['TouchableWithoutFeedback', TouchableWithoutFeedback, {}],
+    ['TouchableNativeFeedback', TouchableNativeFeedback, {}],
+    ['Pressable', Pressable, {}],
+  ])('handles disabled prop for %s', (_, Component, props) => {
+    const { queryByTestId } = render(
+      <Component disabled testID="touchable" {...props}>
+        <View />
+      </Component>,
+    );
+
+    expect(queryByTestId('touchable')).toBeDisabled();


Suggested change

test.each([

['Button', Button, { title: 'some button' }],

['TouchableOpacity', TouchableOpacity, {}],

['TouchableHighlight', TouchableHighlight, {}],

['TouchableWithoutFeedback', TouchableWithoutFeedback, {}],

['TouchableNativeFeedback', TouchableNativeFeedback, {}],

['Pressable', Pressable, {}],

])('handles disabled prop for %s', (_, Component, props) => {

const { queryByTestId } = render(

<Component disabled testID="touchable" {...props}>

<View />

</Component>,

);

expect(queryByTestId('touchable')).toBeDisabled();

test.each([

['TouchableOpacity', TouchableOpacity, {}],

['TouchableHighlight', TouchableHighlight, {}],

['TouchableWithoutFeedback', TouchableWithoutFeedback, {}],

['TouchableNativeFeedback', TouchableNativeFeedback, {}],

['Pressable', Pressable, {}],

])('handles disabled prop for %s', (_, Component, props) => {

const { getByText, getByTestId } = render(

<Component disabled testID="touchable" {...props}>

<Text>Trigger</Text>

</Component>,

);

expect(getByText('Trigger')).toBeDisabled();

expect(getByTestId('touchable')).toBeDisabled();

I think that we should assume that there is a certain duality between RNTL fireEvent and toBeDisabled, if fireEvent calls and event handler then toBeDisabled should be false, and vice-versa (obviously except the case when there is no event handler defined). So if RNTL users frequently use things like getByText to find button with given text, we should also support calling toBeDisabled() on that getByText result.

@brunohkbx @thymikee Wdyt? Does it make sense? (my RNTL expertise got little rusty lately)

NB: Button is excluded because it uses title prop and didn't want to mix it with child element. We can test it separately if needed.

mdjastrzebski · 2021-04-08T20:44:56Z

src/__tests__/to-be-disabled.js

-      expect(queryByTestId(name)).toBeDisabled();
-      expect(() => expect(queryByTestId(name)).not.toBeDisabled()).toThrowError();
-    });
+  test.each([


We probably want to test the opposite case as well (not.toBeDisabled() when there is no disabled prop).

mdjastrzebski · 2022-09-30T12:20:40Z

This PR is out of sync with main branch.

@brunohkbx would you have time and will to re-examine the PR and rebase/reimplement the relevant parts with the current codebase?

mdjastrzebski · 2022-11-04T08:40:52Z

Closing as stale. Things have move a lot since this PR has been create. @brunohkbx We might restart this effort in a fresh PR, based on current codebase, if you still see value in it.

brunohkbx added the breaking change label Nov 12, 2020

brunohkbx requested review from mdjastrzebski and thymikee November 12, 2020 15:42

brunohkbx changed the title ~~BREAKING CHANGE: support @testing-library/react-native 7.0~~ feat: support @testing-library/react-native 7.0 Nov 12, 2020

brunohkbx force-pushed the feat/support-@testing-library/react-native-7.0 branch 2 times, most recently from 0accbc4 to 6978fcb Compare November 12, 2020 15:57

brunohkbx force-pushed the feat/support-@testing-library/react-native-7.0 branch from 6978fcb to 2835556 Compare April 6, 2021 19:39

feat: support @testing-library/react-native 7.0

1f1bda8

brunohkbx force-pushed the feat/support-@testing-library/react-native-7.0 branch from 2835556 to 1f1bda8 Compare April 6, 2021 19:42

brunohkbx mentioned this pull request Apr 6, 2021

.toBeDisabled() buggy with RN 0.62 #23

Closed

mdjastrzebski reviewed Apr 8, 2021

View reviewed changes

fabiendem mentioned this pull request Oct 22, 2021

Support @testing-library/react-native 7.0 #43

Closed

mdjastrzebski closed this Nov 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: support @testing-library/react-native 7.0 #50

feat: support @testing-library/react-native 7.0 #50

Uh oh!

brunohkbx commented Nov 12, 2020 •

edited

Loading

Uh oh!

codecov bot commented Nov 12, 2020 •

edited

Loading

Uh oh!

thymikee commented Nov 12, 2020

Uh oh!

brunohkbx commented Nov 17, 2020

Uh oh!

deriegle commented Dec 18, 2020 •

edited

Loading

Uh oh!

brunohkbx commented Apr 6, 2021 •

edited

Loading

Uh oh!

mdjastrzebski left a comment

Uh oh!

mdjastrzebski Apr 8, 2021

Uh oh!

mdjastrzebski Apr 8, 2021

Uh oh!

mdjastrzebski Apr 8, 2021

Uh oh!

mdjastrzebski commented Sep 30, 2022

Uh oh!

mdjastrzebski commented Nov 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

feat: support @testing-library/react-native 7.0 #50

feat: support @testing-library/react-native 7.0 #50

Uh oh!

Conversation

brunohkbx commented Nov 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

thymikee commented Nov 12, 2020

Uh oh!

brunohkbx commented Nov 17, 2020

Uh oh!

deriegle commented Dec 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brunohkbx commented Apr 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mdjastrzebski left a comment

Choose a reason for hiding this comment

Uh oh!

mdjastrzebski Apr 8, 2021

Choose a reason for hiding this comment

Uh oh!

mdjastrzebski Apr 8, 2021

Choose a reason for hiding this comment

Uh oh!

mdjastrzebski Apr 8, 2021

Choose a reason for hiding this comment

Uh oh!

mdjastrzebski commented Sep 30, 2022

Uh oh!

mdjastrzebski commented Nov 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

brunohkbx commented Nov 12, 2020 •

edited

Loading

codecov bot commented Nov 12, 2020 •

edited

Loading

deriegle commented Dec 18, 2020 •

edited

Loading

brunohkbx commented Apr 6, 2021 •

edited

Loading