Add skip display none #174
Conversation
|
I don't know why github shows 16 commits. |
|
@tangrufus The reason is that you did not sync up your remote to our latest. Please sync up, and rebase your commits to one. Thanks. |
| dom_id: dom_id | ||
| }) | ||
| component_specification_tag_options = { | ||
| class: "js-react-on-rails-component", |
There was a problem hiding this comment.
Please remove any extraneous whitespace changes.
|
rebased & |
|
Passed the tests finally. |
| expect_turbolinks: turbolinks_loaded, | ||
| dom_id: dom_id | ||
| }) | ||
| content_tag(:div, |
Thanks! Looks great overall. |
|
How do you test the initializer config? |
|
I think you can probably use a capybara test and set the initial value at the beginning of the test. Take a look at https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/spec/features/integration_spec.rb#L100 |
|
@tangrufus If you can get a spec in, I'll merge this. |
|
@tangrufus Easiest way would be to add it to the spec/dummy app as an integration test |
|
@tangrufus I think you have one test to test the default value of the option here https://github.com/shakacode/react_on_rails/blob/master/spec%2Freact_on_rails%2Fconfiguration_spec.rb and put another test under dummy app to verify the feature is working as expected. |
|
@tangrufus We can put this one in after we release 2.0 if that's OK with you. We need a test for this. |
|
Sorry for the delay. Does anyone can help writing the tests? @justin808 No problem. Merge only if you think it's ready. |
|
@tangrufus Once this PR is rebased on top of master and has a few tests, we'll merge. |
|
@tangrufus Not sure where you are with this but I can help with writing the tests. |
|
@aaronvb, go ahead! |
|
Thanks @aaronvb |
|
@tangrufus @justin808 What's the best way for me to work on the tests other than giving me access to your repo? |
|
@aaronvb merge @tangrufus change onto a branch for @shakacode and continue from there. |
|
@justin808 @tangrufus I've pushed a branch with the tests added: https://github.com/shakacode/react_on_rails/tree/TangRufus-add-skip-display-none |
|
Please create a PR and let me know when it's ready for merge, @aaronvb. Thanks!! 👏 |
|
closing this in favour of #218 |
Resolve #173