Skip to content

Fix centerContent on ScrollView#2332

Closed
nandorojo wants to merge 2 commits intonecolas:masterfrom
nandorojo:center-content
Closed

Fix centerContent on ScrollView#2332
nandorojo wants to merge 2 commits intonecolas:masterfrom
nandorojo:center-content

Conversation

@nandorojo
Copy link
Contributor

@nandorojo nandorojo commented Jul 6, 2022

Closes #2331.

I tried to naïvely add a unit test, but I don't have experience writing them, so I'm clearly missing something.

I attempted this:

  describe('prop "centerContent"', () => {
    test('without', () => {
      const { container } = render(
        <ScrollView style={{ backgroundColor: 'blue' }} />
      );
      expect(container.firstChild).toMatchSnapshot();
    });

    test('with', () => {
      const { container } = render(
        <ScrollView centerContent style={{ backgroundColor: 'blue' }} />
      );

      expect(container.firstChild).toMatchSnapshot();
      expect(container.firstChild).toHaveStyle('justify-content', 'center');
      expect(container.firstChild).toHaveStyle('flex-grow', '1');
    });
  });

However, it seems that toHaveStyle doesn't exist. I came across that API from searching online for jest-dom + @testing-library/react, but I wasn't really sure where to go from there.

I've tested it in my app and it's working well for me. Happy to add/update a unit test if you have any guidance for updating my broken test.

@nandorojo nandorojo changed the title Fix centerContent on ScrollView; close #2331 Fix centerContent on ScrollView Jul 6, 2022
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 6, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 34e2d86:

Sandbox Source
react-native-web-examples Configuration

collapsable={false}
ref={this._setInnerViewRef}
style={StyleSheet.compose(
style={[
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Every example of StyleSheet.compose throughout the repo had only two styles passed to it. Since this one now has three, I turned it into an array. If that's incorrect, feel free to let me know.

@burakgormek
Copy link
Contributor

IMO for the test, toMatchSnapshot is enough. It will add classes to div and will compare output to match snapshot. No needed to check with toHaveStyle.

But you have to run test to create initial snapshot.

@nandorojo
Copy link
Contributor Author

IMO for the test, toMatchSnapshot is enough. It will add classes to div and will compare output to match snapshot. No needed to check with toHaveStyle.

But you have to run test to create initial snapshot.

Thanks for the feedback. Didn’t realize it was that trivial. I pushed the test without toHaveStyle.

@necolas necolas closed this in 80ef32f Jul 6, 2022
rnike pushed a commit to VeryBuy/react-native-web that referenced this pull request Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ScrollView's centerContent prop doesn't work

2 participants