Skip to content

bug(CSSTransition): pass 'appearing' param from Transition to CSSTransition#144

Closed
alonbt wants to merge 2 commits into
reactjs:masterfrom
alonbt:pass-appearing-to-css-transition
Closed

bug(CSSTransition): pass 'appearing' param from Transition to CSSTransition#144
alonbt wants to merge 2 commits into
reactjs:masterfrom
alonbt:pass-appearing-to-css-transition

Conversation

@alonbt

@alonbt alonbt commented Aug 3, 2017

Copy link
Copy Markdown
Contributor

No description provided.

@jquense

jquense commented Aug 3, 2017

Copy link
Copy Markdown
Contributor

awesome thanks!

Comment thread test/CSSTransition-test.js Outdated
onEntered(node, appearing){
expect(node.className).toEqual('');
expect(count).toEqual(2);
expect(appearing).toBe(false);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could you also add a test confirming that it's true when appearing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I gave it a try but I'm not sure how to do it simple or how to do it at all. I tried to find an example for mounting a component but couldn't find on that file. I tried to do something like this but it didn't work:


    it.only('should apply pass isAppearing true', () => {

      let appearing = {}

      jest.resetModuleRegistry();
      jest.useFakeTimers();

      const container = document.createElement('div');
      const ReactDOM = require('react-dom');
      class Yolo extends React.Component {

        onEnter(node, isAppearing) {
          appearing.onEnter = isAppearing;
        }

        render() {
          return (
            <CSSTransition classNames="yolo" appear={true} enter={true} timeout={10} onEnter={this.onEnter}>
              <span>Spanning around</span>
            </CSSTransition>
          )
        }
      }

      ReactDOM.render(
        <Yolo/>,
        container,
      );

      jest.runAllTimers();

      expect(appearing.onEnter).toBe(true); // returns undefined === onEnter wasn't even called

    });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

follow the same pattern as the Entering test, but where instance is

instance = tsp(
        <CSSTransition
          timeout={10}
          appear={true}
          in={true}
          classNames="test"
        >
          <div/>
        </CSSTransition>
      )
      .render();

@alonbt alonbt force-pushed the pass-appearing-to-css-transition branch from 6146d27 to 93cec5d Compare August 3, 2017 21:52
});
});

describe('isAppearing', () => {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I refactored it to create 2 new tests - one for false and one for true

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it was also a bit confusing that this didn't work for the false part (because this pattern did work for the 'true' part):

it('should be false', done => {

        instance = tsp(
          <CSSTransition
            timeout={10}
            classNames="test"
            in={true}
          >
            <div/>
          </CSSTransition>
        )
          .render();


        instance.props({
          /* no in:true goes here..*/
         onEnter() ....

@Lazyuki Lazyuki mentioned this pull request Dec 20, 2018
jquense pushed a commit that referenced this pull request Dec 20, 2018
Based on #144, but with working tests. Fixes #143.
@silvenon

silvenon commented Apr 6, 2019

Copy link
Copy Markdown
Collaborator

Fixed in df7adb4.

@silvenon silvenon closed this Apr 6, 2019
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.

3 participants