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

Typescript Support #52

Merged
merged 21 commits into from
Nov 3, 2019
Merged

Typescript Support #52

merged 21 commits into from
Nov 3, 2019

Conversation

BlackFenix2
Copy link
Contributor

I converted the ReactCardFlip component from javascript to typescript. i also had to modify the jest config to handle .ts and .tsx files.

i also bumped the entire package.json to the latest dependencies.

let me know if you have any questions.

@BlackFenix2 BlackFenix2 mentioned this pull request Oct 12, 2019
@BlackFenix2
Copy link
Contributor Author

i also removed the need to provide a 'key' attribute to the children of the ReactCardFlip Component. I refactored the getComponent function to find the front and back element by children's array index rather than an attribute.

const getComponent = (key: 0 | 1) => {
    if (props.children.length !== 2) {
      throw new Error(
        'Component ReactCardFlip requires 2 children to function'
      );
    }
    return props.children[key];
  };

so now you don't need to specify 'key' anymore.

<ReactCardFlip isFlipped={this.state.isFlipped}>
        <div style={this.props.styles.card}>
          <img
            style={this.props.styles.image}
            src="//static.pexels.com/photos/59523/pexels-photo-59523.jpeg"
          />

          <button onClick={this.handleClick}>Flip Card</button>
        </div>

        <div style={this.props.styles.card}>
          <img
            style={this.props.styles.image}
            src="//img.buzzfeed.com/buzzfeed-static/static/2014-04/enhanced/webdr06/4/16/enhanced-11136-1396643149-13.jpg?no-auto"
          />

          <button onClick={this.handleClick}>Flip Card</button>
        </div>
      </ReactCardFlip>

BlackFenix2 and others added 2 commits October 12, 2019 00:07
Accidentally added react and react-dom to dependencies, removing.
@AaronCCWong
Copy link
Owner

Thanks for working on this! I will review asap.

Copy link
Owner

@AaronCCWong AaronCCWong left a comment

Choose a reason for hiding this comment

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

Overall looks great! Just a small nitpick. Lets stick with npm.

package.json Outdated Show resolved Hide resolved
@BlackFenix2
Copy link
Contributor Author

I reverted the changes to package.json to use npm. all of the tests run fine locally. Let me know if you need anything else.

Copy link
Contributor Author

@BlackFenix2 BlackFenix2 left a comment

Choose a reason for hiding this comment

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

I reviewed the changes

@AaronCCWong
Copy link
Owner

I reviewed the changes

Can you fix the files that have merge conflicts? Thanks!

@BlackFenix2
Copy link
Contributor Author

oops, still kinds newb-ish in my Git commands. i fixed all of the merge conflicts.

@AaronCCWong AaronCCWong merged commit 7b66f6b into AaronCCWong:master Nov 3, 2019
AaronCCWong pushed a commit that referenced this pull request Nov 7, 2019
* added quote_type = single to .editorconfig for lint/preetier consistency

* prettier formatting, added react settings to eslintrc

* rewrote component with hooks

* added wrapper.update() method to catch prop updates on function component

* added typescript. build cmd works but webpack.e2e.js doesnt work

* added typescript

* fixed extensions option in webpack configs from 'tsx' to '.tsx'

* finished scaffolding typescript and jest. added jsdoc to ReactCardFlip.tsx properties.

* added build folders to .gitignore

* removed lib folder

* hotfix: throw an exception if the component does not have exactly 2 children.

* removed the key attribute requirement for ReactFlipCard children

* hotfix: forgot to include index.d.ts in package.json file array

* Update package.json

Accidentally added react and react-dom to dependencies, removing.

* replaced yarn scripts with npm, removed yarn.lock and trimmed jest.config.js

* modified index.d.ts

* Add unit test for vertical flipping and fix prop deconstruct

* Remove prop-types install from travis

* Update README to reflect removal of key

Closes #51
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.

None yet

2 participants