ifad / data-confirm-modal

Makes Rails' link_to confirm: 'foo' build a Bootstrap Modal instead of calling the browser's confirm() API.
MIT License
270 stars 115 forks source link

Rails 6 compatibility (webpacker) #77

Closed mattes closed 4 years ago

mattes commented 4 years ago

This gem checks if if (window.Rails || $.rails) are defined. Which doesn't appear to be the case for Rails 6.

I updated app/javascript/packs/application.js to make window.Rails globally available:

const Rails = require("@rails/ujs")
Rails.start()
window.Rails = Rails

Hope it helps.

vjt commented 4 years ago

Thank you for this.

What would be the best way to check if Rails’ ujs is available with Rails 6, while maintaining backwards compatibility?

I don’t like to suggest users to pollute the global namespace :-).

Thanks,

ksouthworth commented 4 years ago

+1 for a way to make this Rails 6 (webpacker) compatible without adding a window.Rails global object

countalucard2022 commented 4 years ago

This gem checks if if (window.Rails || $.rails) are defined. Which doesn't appear to be the case for Rails 6.

I updated app/javascript/packs/application.js to make window.Rails globally available:

const Rails = require("@rails/ujs")
Rails.start()
window.Rails = Rails

Hope it helps.

Hmmm, using rails 6 too and not working.

Not sure if its the way data confirm modal was imported.

I just follow the instruction to add this //= require data-confirm-modal in the app/javascript/packs/application.js

and also added your suggestion. I don't have error in the console or anything it's just not working. Any work around?

tagliala commented 4 years ago

I do not have write permissions to push a fix on the readme, but I can confirm that this gem works on Rails 6 / Webpacker.

Instructions:

$ yarn add data-confirm-modal

Then add to app/javascript/packs/application.js after require('@rails/ujs').start()

require('data-confirm-modal')

data-confirm-modal gem is not necessary for webpacker usage

Example: diowa/ruby2-rails6-bootstrap-heroku@1a59af4

vjt commented 4 years ago

@tagliala you know have access :-)

countalucard2022 commented 4 years ago

I do not have write permissions to push a fix on the readme, but I can confirm that this gem works on Rails 6 / Webpacker.

Instructions:

$ yarn add data-confirm-modal

Then add to app/javascript/packs/application.js after require('@rails/ujs').start()

require('data-confirm-modal')

data-confirm-modal gem is not necessary for webpacker usage

Example: diowa/ruby2-rails6-bootstrap-heroku@1a59af4

will try the solution, thanks alot!

countalucard2022 commented 4 years ago

@tagliala ,

Added the suggested steps

application.js

import 'bootstrap'
require("@rails/ujs").start()
require('data-confirm-modal')
require("turbolinks").start()
require("@rails/activestorage").start()
require("channels")

package.json:

    "@rails/actioncable": "^6.0.0",
    "@rails/activestorage": "^6.0.0",
    "@rails/ujs": "^6.0.0",
    "@rails/webpacker": "4.2.2",
    "bootstrap": "4.3.1",
    "data-confirm-modal": "^1.6.2",

but still on the the data-confirm still not showing up in modal, not sure which part did I made a mistake..

tagliala commented 4 years ago

can you please share a github repo with the minimum amount of code to reproduce?

You may need some configuration from https://github.com/diowa/ruby2-rails6-bootstrap-heroku/blob/master/config/webpack/environment.js

countalucard2022 commented 4 years ago

Ahhh, that's the cherry on top of ice cream, I've readded the same config you shared on my environments.js and it worked. It kinda mess my current modals but it's definitely working, thanks alot.

tagliala commented 4 years ago

You're welcome

@countalucard2022 could you please share your previous configuration? did you expose jquery?

countalucard2022 commented 4 years ago

@tagliala not completely related here. I pasted the config you have but before that I have these:

environment.plugins.prepend('Provide', new webpack.ProvidePlugin({
  $: 'jquery/src/jquery',
  jQuery: 'jquery/src/jquery',
  jquery: 'jquery',
  'window.jQuery': 'jquery',
  Popper: ['popper.js', 'default']
}))

Yours have : ( on difference perspective)

$: 'jquery',
    jQuery: 'jquery',
    jquery: 'jquery',

Possible explain what's the difference? I'm calling jquery/src/jquery to handle my custom modals, (rendering my partials to modal) When I enter the solution for data-confirm-modal I'm getting this error: Uncaught ReferenceError: $ is not defined

If I removed that part and keep mine I'm getting this:

data-confirm-modal.js:80 Uncaught Error: The bootstrap modal plugin does not appear to be loaded.
    at data-confirm-modal.js:80

This is my test repo

tagliala commented 4 years ago

Disclaimer: I'm not a node/javascript developer

Possible explain what's the difference?

ProvidePlugin is needed to automatically load the default export of the module, without the need to require it in every single javascript file.

In our case, we should autoload the same object for $, jquery, jQuery, and window.jQuery. It is important that they are the same because if you inspect your compiled application.js with mixed values, you can see that jquery has been included twice.

The difference is that with jquery/src/jquery you are loading jquery from the sources, and that should be fine, but with jquery you are loading from the main entrypoint file defined in package.json, which is dist/jquery.js for jquery

Webpack advises to use the main entrypoint use case: https://webpack.js.org/plugins/provide-plugin/#usage-jquery

I'm calling jquery/src/jquery to handle my custom modals,

Got it. Your application is loading a JavaScript file that contains $(...), but webpack by default do not expose $, so you get an uncaught reference error. By loading jquery from the sources, you got the side effect to exposing it to window

If you want to use $ outside webpacker, just expose it (add window.$ = jquery in application.js)

countalucard2022 commented 4 years ago

@tagliala , thanks for the patience and explanation, all clear now. I will adjust my practice to fit on the hidden jquery, all worked fine now

vjt commented 4 years ago

Thanks @tagliala. I believe a note on the README would be appropriate to not confuse other potential users.

Care to add some instructions to it?

Thank you,