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

[WIP] Refine API #24

Merged
merged 7 commits into from
Oct 2, 2015
Merged

[WIP] Refine API #24

merged 7 commits into from
Oct 2, 2015

Conversation

vramana
Copy link
Contributor

@vramana vramana commented Sep 28, 2015

  • Mixins instead of Middlewares
  • Fix lint errors on tests
  • Remove helpers
  • Remove use as its obselete.
  • Deprecate test (???)
  • Simulate Keypress on input mixin ( changeInput / inputEnter)
  • nested find using querySelectorAll (???)

- Refactored Test Class into function
- Removed React from being a global variable
- Added babelrc
- Split npm test into npm run mocha and npm run lint
elements can be accesed by this.elements
middleware
no-dom.js
tests.js
lib
Copy link
Member

Choose a reason for hiding this comment

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

It still needs to output files to the root directory

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 does out to the root directory. When I switched branched, old lib folder stayed there and it was removed from gitignore So, I added it gitignore instead of deleting those lib files. I'll remove that line.

Copy link
Member

Choose a reason for hiding this comment

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

👍

@geowarin
Copy link

I really like this PR. Shall I close #23 and make a new one once this gets merged and we decide on an API for nested search?

@zackify
Copy link
Member

zackify commented Sep 29, 2015

Okay this is super awesome! Really really like these improvements! I want to merge this badly! Can you just do two things: Add the mixin stuff to the readme and also add a test file for mixins. Then I'll be merging :) @geowarin I think you should do that. Thank you both for all your work!!

@zackify
Copy link
Member

zackify commented Sep 29, 2015

Oh and make sure if you pass just a callback it still just grabs the first element

}

if (Object.keys(this.elements).length === 1) {
select.call(this, this.elements[Object.keys(this.elements)[0]])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zackify This line grabs the callback.

@vramana
Copy link
Contributor Author

vramana commented Sep 29, 2015

I want to do a bunch a little stuff before this gets I'll do it by tomorrow.

@zackify
Copy link
Member

zackify commented Sep 30, 2015

What more do you want to wait on?

@vramana
Copy link
Contributor Author

vramana commented Sep 30, 2015

@zackify Nevermind. I'll do it another PR. I kind of forgot what I wanted to do cuz I am a little busy until the weekend. I'll take a look on Friday again.

@NickStefan
Copy link
Contributor

👍 cant wait to use this!

@zackify
Copy link
Member

zackify commented Oct 2, 2015

@vramana any updates?

@vramana
Copy link
Contributor Author

vramana commented Oct 2, 2015

@zackify Merge this. I did the changes you asked for. I'll submit another PR tomorrow.

// In this example, CustomFind middleware was added to Test by mixin
// and used if as it was a method on Test itself.

Test(<TestComponent />)
Copy link
Member

Choose a reason for hiding this comment

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

would be cool to do something like Test.mixin at the top of your tests file or in a setup file and then have them available everywhere.

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 will do that in the next PR. I am really tired today! Had a long day!!

zackify added a commit that referenced this pull request Oct 2, 2015
@zackify zackify merged commit 19c6843 into Legitcode:master Oct 2, 2015
This pull request was closed.
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.

4 participants