Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed

Conversation

alonbt
Copy link
Contributor

@alonbt alonbt commented Aug 3, 2017

No description provided.

@jquense
Copy link
Collaborator

jquense commented Aug 3, 2017

awesome thanks!

expect(node.className).toEqual('');
expect(count).toEqual(2);
expect(appearing).toBe(false);
Copy link
Collaborator

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
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
Collaborator

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
@@ -112,6 +112,70 @@ describe('CSSTransition', () => {
}
});
});

describe('isAppearing', () => {
Copy link
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
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
Copy link
Collaborator

silvenon commented Apr 6, 2019

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