Add disabled support for pt-file-upload

[melonhead901]

When pt-file-upload is disabled the following changes should take place:

1. Input box appears disabled.
2. "Browse" button does not open a file upload box.
3. "Browse" button appears disabled.

Change 1 can be accomplished with .pt-disabled on the form input. Change 2 can be accomplished with a disabled attribute on the input as well. Change 3 requires custom CSS.

Would be nice if these could all be accomplished with a single disabled flag.

[llorca]

Sounds good, we'll add support for both :disabled and .pt-disabled as we do for other things. Note that .pt-disabled will only make it look disabled. :disabled will make it look disabled and will disable the interactions (i.e. opening a file upload modal)

[cmslewis]

@llorca This looks tricky: .pt-file-upload is composed of an input[type="file"] and a custom-styled <span>. As such, we don't get disabled styles for free when we apply pt-disabled or :disabled to the wrapper; we'd have to expose disabled styles via new mixins (e.g. pt-button-disabled()), then apply those styles to the child elements iff the wrapper is decorated with .pt-disabled or :disabled.

Also, since .pt-file-upload is implemented as a <label>, it doesn't appear to support the "disabled" attribute at all (at least, I was having problems getting those styles to show up when I that attribute was present).

Also also, the fact that the input is wrapped in a <label> makes it difficult to disable click effects entirely. Even setting pointer-events: none on the input[type="file"] element doesn't work, because a click on the <label> still works (and you can't set point-events:none on the <label>; otherwise the cursor won't display as not-allowed, breaking styling consistency with other disabled components).

If you can somehow set the input[type="file"] as :disabled, you get this:

...but it's not possible to do that with the desired CSS API. We'd have to instruct users to set pt-disabled on pt-file-upload and disabled on the child input.

Here are the styles for the example above by the way, complete with new mixins:

.pt-file-upload {
  // ...

  // .pt-file-upload is implemented as an HTML label,
  // and :disabled is not supported on label elements.
  &.pt-disabled {
    .pt-file-upload-input {
      @include pt-input-disabled();

      &::after {
        @include pt-button-disabled();
      }

      .pt-dark & {
        @include pt-dark-input-disabled();

        &::after {
          @include pt-dark-button-disabled();
        }
      }
    }
  }
}

[llorca]

Thanks for researching! I have a few questions:

• Are you saying we already support disabled styles & interactions, but with :disabled on .pt-file-upload input input[type="file"] only?
• I can't remember why .pt-file-upload is a <label>, it should be a <div> wrapper than you can then put into a <label> with some text, similar to how other form controls work. Would that help us get pointer-events: none for .pt-file-upload.pt-disabled?
• Side question: have we ever considered applying pointer-events: none globally on all .pt-disabled things? Seems like it could help .pt-disabled get closer to the native :disabled behavior.

[cmslewis]

Are you saying we already support disabled styles & interactions, but with :disabled on .pt-file-upload input input[type="file"] only?

Ehh, short answer: no. Long answer: let me rephrase. Here's the markup:

• input[type="file"] is visually hidden; the surrounding label accepts clicks and forwards them down into this input.
• .pt-file-upload-input is styled as a pt-input using the pt-input mixin.
  Adding .pt-disabled already adds disabled styles to this input. Adding :disabled won't work though, because spans can't be disabled.
• .pt-file-upload-input::after is styled as a pt-button using the pt-button mixin.
  But it's not possible to add additional classes or attributes to pseudo-elements, so you can't make this look disabled right now.

I'd intuitively want to disable the File Upload component by adding :disabled or .pt-disabled to the parent:

<label class="pt-file-upload pt-disabled">
// or 
<label class="pt-file-upload" disabled>

Problems:

• label:disabled isn't a thing, so disabled styles won't show up for that selector.
• label.pt-disabled can show disabled styles if you introduce additional CSS rules that percolate disabled stylings to child elements (preferably using new pt-input-disabled and pt-button-disabled mixins). e.g. you could style .pt-file-upload.pt-disabled > .pt-file-upload-input
• To actually disable the click functionality, there are two choices:
  • Add pointer-events: none to the label, sacrificing cursor: not-allowed styling on hover (cursors stylings are not respected when pointer-events is set to none).
  • Add a disabled attribute to the input[type="file"] in addition to adding a pt-disabled class on the parent element. This is what I did in my previous comment; it retains the not-allowed cursor styling.

Summary:

• The optimal end-user experience has this API:

• Whereas the optimal developer experience has this API:

• Addendum: a hybrid approach that would create a nice end-user experience AND developer experience, is to use CSS sibling selectors to add disabled styles when just the input is disabled (e.g. input[type="file"]:disabled + .pt-file-upload-input). That would look like this:

I can't remember why .pt-file-upload is a <label>, it should be a <div> wrapper than you can then put into a <label> with some text, similar to how other form controls work. Would that help us get pointer-events: none for .pt-file-upload.pt-disabled?

I'd imagine the label made the click area span the whole component. But yeah let's make it a div and just make the invisible input[type="file"] the full 30px in height (it's currently 20-something px tall):

That has no bearing on disabling click events though (cursor styles still won't look right with pointer-events: none).

Side question: have we ever considered applying pointer-events: none globally on all .pt-disabled things? Seems like it could help .pt-disabled get closer to the native :disabled behavior.

We can't do this if we also want to support cursor: not-allowed. But if we want to do away with that styling, I'm fully in favor of styling all disabled elements with pointer-events: none, yes.

[cmslewis]

Ooh, here are two issues from Bootstrap about pointer-events: none and cursor: not-allowed when a component is .disabled:

twbs/bootstrap#17703
twbs/bootstrap#16088

Interesting reads.

[llorca]

Good stuff. @leebyp @giladgray any opinion on the research above?

[leebyp]

• I don't see anything bad about the proposed best of both worlds - it just requires specific css on our end. As mentioned, I would guess that the label reasoning is just because that's how the other forms worked before the introduction of form groups.

• The cursor: not-allowed is pretty vital so let's leave as is. We can open a new thread around how we style disableds, especially since we've started using important in some places.

[cmslewis]

I'll push a fix based on the hybrid approach. Happy to chat more in the PR.

[cmslewis]

Checkboxes actually already do the hybrid approach for disabling, so we have precedent in our own codebase:

👍

[giladgray]

i'm onboard with what you've done in the PR, @cmslewis. nice work!