ifad / data-confirm-modal

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

options.text vs options.verifyLabel #73

Closed zeroedin closed 5 years ago

zeroedin commented 5 years ago

options.text is added to modal as follows:

$.each((options.text||'').split(/\n{2}/), function (i, piece) {
  body.append($('<p/>').html(piece));
});

However options.verifyLabel is added like:

if (options.verifyLabel)
  body.append($('<p>', {text: options.verifyLabel}))

Could both of these inserts be normalized? I am trying to add a class wrapper around some text in these inserted paragraphs to word break their content as my file names tend to be long and I'd rather not make the modal bigger if possible, instead just word breaking using css. If possible I'd like to do this specifically on file name rather then targeting the parent paragraph, as I don't want the the paragraphs to break on less long words.

confirm: "Are you sure you want to delete <span class='confirm-filename'>`#{file.filename}`</span>?  This is a permanent action an can not be undone.",
verify: "#{entity.filename}",
verify_text: "Type <span class='confirm-filename'>`#{file.filename}`</span> to confirm"},`

Could be fixed by using .html() instead of .. {text: 'options.verifyLabel'}

$.each((options.text||'').split(/\n{2}/), function (i, piece) {
  body.append($('<p/>').html(piece));
});

if (options.verifyLabel)
  body.append($('<p>').html(options.verifyLabel))

Let me know if there is any issues with this approach?

zeroedin commented 5 years ago

Closing this issue, just realized i could eliminate my usage of the verify text and do this:

confirm: "Are you sure you want to delete <span>`#{file.filename}`</span>? \n\n Type <span>`#{file.filename}`</span> to confirm \n\n <strong>This is a permanent action and can not be undone.</strong>",

It does however bring up another issue. Should verifyText create a <label> instead of a <p> tag? I'll open a separate issue for that #74