Skip to content

Fix #197 #232

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

Merged
merged 2 commits into from
Jan 25, 2016
Merged

Fix #197 #232

merged 2 commits into from
Jan 25, 2016

Conversation

frontsideair
Copy link
Contributor

Removes jQuery and bootstrap.js dependencies, but adds react-onclickoutside (which is a good tradeoff). I removed those scripts from examples/index.html for good measure.

@@ -48,6 +48,7 @@
"react-bootstrap": "^0.27.2",
"react-dom": "^0.14.3",
"react-hot-loader": "^1.3.0",
"react-onclickoutside": "^4.5.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it should be a dependency and not a dev-dependency?

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 don't know. I thought since it's small enough, we don't need to increase the number of runtime dependencies and could just bundle it. I can change it if you think it makes a difference.

Copy link
Owner

Choose a reason for hiding this comment

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

I thinks it should be a dependency :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I'll change it to da dependency while I'm on it.

@frontsideair
Copy link
Contributor Author

Should be okay now, please review.

AllenFang added a commit that referenced this pull request Jan 25, 2016
@AllenFang AllenFang merged commit 47dab73 into AllenFang:master Jan 25, 2016
@AllenFang AllenFang mentioned this pull request Jan 25, 2016
@AllenFang
Copy link
Owner

@frontsideair, I found not only dropdown menu for pagination but also insert row for popup a modal will also use bootstrap.js. sorry, this is my fault, could you please also fix the insert button for your solution. Because I need to release today, so I revert this PR and will merge it in next version. Sorry

@frontsideair
Copy link
Contributor Author

No problem, I'll take a look as soon as possible. If there's anyplace else using bootstrap.js you can add a comment and I'll check them too.

@AllenFang
Copy link
Owner

@frontsideair, take your time, currently I just know the insert button for popup modal and the close button in modal. Thanks :)

@DongShi
Copy link

DongShi commented Mar 6, 2016

Hello there, by now, this library still depends on bootstrap.js and jquery right?
I just port one demo code to my own project and found that the dropdown list for pagination did not work.

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