Allow default option for disabled-with part two

[abitdodgy]

Rails has a great feature for translating button text. We can write this:

/posts/_form.html.slim
= f.submit class: 'button'

This feature allows us to reuse partials in new and edit templates with no logic. Rails automatically substitutes the button text based on your en.yml file.

en:
  helpers:
    submit:
      post:
        create: "New"
        update: "Save"

Now, if you want to disable the button state and still reuse your partial, you have to use some ugly logic:

- if @post.new_record?
  = f.submit class: 'btn', 'data-disable-with' => t('helpers.submit.post.create')
- else
  = f.submit class: 'btn', 'data-disable-with' => t('helpers.submit.post.update')

Which defeats the point. It would be nice to have a a disabled-with-default option that just disables the button with its existing text. Something like:

= f.submit class: 'button', 'disable-with-default'

[lucasmazza]

I wonder if we can achieve something like this without adding a new data attribute - maybe cases where data: { disable: true } is used we could fallback to the original text of the element instead.

/cc @rafaelfranca @JangoSteve

[rafaelfranca]

You mean data: { disable_with: true } right?

I think @JangoSteve don't like the idea of using boolean values as data attribute.

If we can't do this in the jquery_ujs side, I also don't want to add this logic to the Rails helpers. I deprecated disable_with options exactly because I don't want to add logic to helpers only to generate cosmetic facilities.

[JangoSteve]

Can someone clarify for me what exactly we're talking about changing in jquery-ujs in this issue? I don't think I quite understand.

For what it's worth, I know it's not the prettiest thing in the world, but the solution you're currently using doesn't have to be as bad as the code you pasted:

= f.submit class: 'btn', 'data-disable-with' => t("helpers.submit.post.#{ @post.new_record? ? "create" : "update" }")

That being said, I'm still unclear what the proposed disabled-with-default would do.

[abitdodgy]

@JangoSteve it would disable the button with its existing text--the default text. It could be called anything really, it's just what made most sense to me at the time. For example...

...
    submit:
      post:
        create: Post it
        update: Save

  = f.submit class: 'btn', data: { "disable-with-default" => true }

That would cause the button to be disabled with "Post it" or "Save" depending if the post is a new record or otherwise, without needing to stick a conditional there.

Being able to use the f.submit helper without having to specify button text (as it falls back to lang.yml), but then having to specify the disable-state text seems unnecessary, especially when the disabled-text can be inferred from the button's existing text.

It's a minor issue, but it seems logical to disable the button with its current text without having to call a translation function and then figure out if it's new/existing record translate. I hope that made sense.

[JangoSteve]

Got it. So you actually don't want it to change the text at all, you just want it to disable the button without modifying the text. I think rather than disable-with-default=true, we could instead just do data-disable (or data-disable="true" if you want, but we'd likely just look for the presence of such an attribute, not at it's value), like this:

<input type="submit" value="Post it" data-disable />

@rafaelfranca To clarify, it's not that I don't like boolean values as data-attributes; I don't like choosing specific strings and changing their type or meaning. When an HTML attribute has a value, the value is always a string. Others had suggested before to specifically look for the string "false" and treat it as the boolean false instead of treating it as the string that it is, which I don't like.

[JangoSteve]

To clarify further on my previous point, it's not the data-some-attribute=true that causes issues, as the string "true" will evaluate to true in JS anyway. It's when someone wants data-some-attribute=false which won't work, because "false" is still a string, and as with any JS "false" will evaluate to true because it's a non-empty string.

[rafaelfranca]

Right. Got it.

[simsalabim]

@JangoSteve @rafaelfranca excuse me for my interruption guys, as I mentioned in #339 jQuery's data already did the job and it treats data-somewhat="false" as boolean false. Using jQuery we admit that true/false/null can be passed through html not as just strings.

[JangoSteve]

@simsalabim You're right. My example is incorrect. I guess I can't distill the issue down as much as I was trying. The full issue is with:

<a data-remote></a>

<a data-remote=true></a>

<a data-remote=false></a>

We want 1 and 2 to both be true as they're both remote links. So in jquery-ujs, we just check to see if the link has the data-remote attribute. The problem is that 3 will also be included in that. Naturally you'd think, no problem, we can just add a check for if $(this).data('remote'), but that would make 1 false, since its data('remote') == undefined (or null I can't remember).

So, we'd have to explicitly check for if $(this).data('remote') !== false, which gets ugly (though not as ugly as if $(this).data('remote') !== "false", which is the thing I was really against).

At the end of the day, if we're checking for !== false instead of !== "false", I'm not as strongly opposed to that, as long as it's jQuery doing the magic "false" => false conversion and not jquery-ujs.

[simsalabim]

@JangoSteve great, I did it exactly the way you described above. Is there any chance #339 will be merged then?

[simsalabim]

BTW, It could have been as simple as if $(this).data('remote') if only jQuery correctly treated HTML5 empty attributes. I sent a separate patch related to this issue. But that behavior has existed since v1.5 and even if it is fixed thousand of sites using jQuery barely will upgrade to its edge version. Thus having additional check is a solution.

[simsalabim]

As per discussion in jQuery issues I was pointed out that data-* attributes are not boolean in HTML5, thus case 1 <a data-remote></a> is invalid. The question is, do you want to support treating it as a boolean behavior?

[lucasmazza]

Besides the data-* semantics discussion, I opened #347 based on the data-disable proposal. Feel free to review it :smile: