seyhunak / twitter-bootstrap-rails

Twitter Bootstrap for Rails 6.0, Rails 5 - Rails 4.x Asset Pipeline
https://github.com/seyhunak/twitter-bootstrap-rails
4.49k stars 997 forks source link

Injectable modals #737

Closed coppolaf closed 10 years ago

coppolaf commented 10 years ago

Seyhunak,

pls see my issue on your repository

https://github.com/seyhunak/twitter-bootstrap-rails/issues/728 (no one has answered to my question, so....)

i've forked and modified the code, going to submit you my modded version,

i'm not a real expert, but this version is working fine for me, the modal now responds to width selection (as specified on bootstrap docs) every part of the modal has an id (with various 'static' suffixes) so different elements may be addressed in jscript for dinamically injecting html code into them the modal body may have a style also (so you can control the modal height!!

pls CAREFULLY VERIFY IT... and feel free to revise and correct if needed!!

i haven't modified relative docs (readme.md) nor other files because i don't exactly know how they work on rails gem packaging.... i've limited my work to the part of code i really know!!

i'll be available for every question you may have for me... hoping to be helpful to the project...

PS: i think you may close the issue #728 as solved if you think!!!

kind regards, francesco

toadkicker commented 10 years ago

Would you mind helping out and writing tests for the modal helper?

coppolaf commented 10 years ago

available?? always!!!

not very expert in testing.... but always available for learning something new!!! (sure, someone more expert than me must review all my work.... just to not publish a rubbish library!!!)

any suggestion?!?! waiting......

PS: many thanks again for your time!!!

regards, francesco

On Wed, Jun 11, 2014 at 3:36 PM, toadkicker notifications@github.com wrote:

Would you mind helping out and writing tests for the modal helper?

— Reply to this email directly or view it on GitHub https://github.com/seyhunak/twitter-bootstrap-rails/pull/737#issuecomment-45741475 .

toadkicker commented 10 years ago

We have tests in the spec/ folder, specifically https://github.com/seyhunak/twitter-bootstrap-rails/blob/master/spec/lib/twitter_bootstrap_rails/modal_helper_spec.rb. You can use those as a starting point. Docs for rspec are https://github.com/rspec/rspec. Thanks for contributing!

coppolaf commented 10 years ago

ok,

i'll read and try to apply.... pls note i'm actually busy in developing a ror app.... so no so much time to spend... but the project is amazing.....

sure i'll contribute....

PS: may i contact you in case of troubles building tests?!?!

many thanks again, francesco

On Wed, Jun 11, 2014 at 6:16 PM, toadkicker notifications@github.com wrote:

We have tests in the spec/ folder. You can use those as a starting point. Docs for rspec are https://github.com/rspec/rspec. Thanks for contributing!

— Reply to this email directly or view it on GitHub https://github.com/seyhunak/twitter-bootstrap-rails/pull/737#issuecomment-45763700 .

toadkicker commented 10 years ago

Of course! Thanks for the PR.

Sent from my iPhone

On Jun 13, 2014, at 2:32 PM, Francesco notifications@github.com wrote:

ok,

i'll read and try to apply.... pls note i'm actually busy in developing a ror app.... so no so much time to spend... but the project is amazing.....

sure i'll contribute....

PS: may i contact you in case of troubles building tests?!?!

many thanks again, francesco

On Wed, Jun 11, 2014 at 6:16 PM, toadkicker notifications@github.com wrote:

We have tests in the spec/ folder. You can use those as a starting point. Docs for rspec are https://github.com/rspec/rspec. Thanks for contributing!

— Reply to this email directly or view it on GitHub https://github.com/seyhunak/twitter-bootstrap-rails/pull/737#issuecomment-45763700 .

— Reply to this email directly or view it on GitHub.

toadkicker commented 10 years ago

This looks good, just please if you can make that one last change and put the classes back on L#5 I'll merge it as soon as I get the message. Apologize for the delay, I have been on a big spike at my work.

coppolaf commented 10 years ago

ok, i'll try to fix it this evening.... but.... i'm sorry not actually having time to write the rpsec side of the modifications i've made... as i said i'm busy building a ror app.... it's taking more than i was expecting (lots of bugs and/or customer mods requests).... you think may be someone may build that for me/us?!?!

pls note also, i'm using a specific version of the original repo on rails 3.x branch inside my app (running on rails 3.x), on this version i've placed the modded modal_helper.rb...

if i try to use the latest version of the gem, i loose all navbar part of my app (i've tested with and without my modifications applied).... all the navbar objects are on page, but all are marked as "display:none"

any suggestion?!?!

thanks in advance, regards, francesco

toadkicker commented 10 years ago

Not sure about the navbar, I am not experiencing that issue with the Teststrap app.

coppolaf commented 10 years ago

ok, see my previous email for details,

here is the latest commit i've done on the branch injectable-modals

https://github.com/coppolaf/twitter-bootstrap-rails/commit/a6452c2197e502c32b45df50aa1d9efe9354a006

many thanks again for your time.... francesco

coppolaf commented 10 years ago

man,

many thanks for the notification.....

now, i'll try this latest version in my app (actually working with the latest bootstrap3 branch commit)

many many thanks again for you time!!!

when i'll have more free time, sure i'll try to contribute (this time applying to rspecs also!!!)

my best regards, francesco

On 07/31/2014 04:20 AM, toadkicker wrote:

Merged #737 https://github.com/seyhunak/twitter-bootstrap-rails/pull/737.

— Reply to this email directly or view it on GitHub https://github.com/seyhunak/twitter-bootstrap-rails/pull/737#event-147523516.

coppolaf commented 10 years ago

tested,

not working with latest master gem.... really i've no time now to troubleshoot now.... i need to close development phase on my app... i'll try to dig deeper inthe next future... well, i'll delete my side of fork, build a new separate repo where to "cristallize" the gem at the commit 1d93c5a7 level (copying it, the replacing the modal_helper.rb with my version), then add this version as reference into the gemfile!!!

many thanks again... i'll provide to update you when i'll find the origin of my issue....

good night!! francesco

toadkicker commented 10 years ago

Fixed it and tests are green now. I was having some issues with string interpolation for setting the id in the header and body, so unfortunately I removed it. If you figure out what Ruby's problem is with using "#{}" let me know =)