Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

md-textfloat: Inaccessible <input> element #547

Closed
ajoslin opened this issue Nov 6, 2014 · 18 comments
Closed

md-textfloat: Inaccessible <input> element #547

ajoslin opened this issue Nov 6, 2014 · 18 comments
Milestone

Comments

@ajoslin
Copy link
Contributor

ajoslin commented Nov 6, 2014

When the developer uses md-textfloat, he/she doesn't have access to the input itself in the template.

What does that mean? It means that things like this won't work as expected:

<md-textfloat ng-blur="doSomethingOnBlur()"></md-textfloat>

This is because the ngBlur directive is listening for the blur event on the md-textfloat container element, not the inner input element itself. This will be the same for the input, focus, etc events.

Then there are input attributes that won't work:

<md-textfloat autocapitalize="off"></md-textfloat>

The developer will expect this to put the autocapitalize="off" attribute on the input, and stop auto-capitalization on mobile. But again, this attribute will stay on the container.

There are potentially hundreds of directives and attributes that could be put on the <md-textfloat> container that the developer will expect to be passed straight to the <input>. We can't manually cover them all.

Fixing This

The best way to fix this would be to use replace: true on the md-textfloat directive, then set the template to be an <input>. This would cause all of the attributes to be passed to a real native <input>.

Then we could use DOM magic to place the <label> element as a sibling, and remove the label when the input is removed.

@ajoslin ajoslin self-assigned this Nov 6, 2014
@ajoslin ajoslin added this to the 0.6 milestone Nov 6, 2014
@ajoslin ajoslin changed the title Inputs & Textareas: Event Bubbling md-textfloat: Inaccessible <input> element Nov 6, 2014
@ajoslin
Copy link
Contributor Author

ajoslin commented Nov 6, 2014

@ThomasBurleson

@ThomasBurleson
Copy link
Contributor

@ajoslin -

  1. Why not just transpose (and remove from originator) all attributes from the md-text-float to the internal <input>. This way the md-text-float container still works to define bounds, ripple, transitions, etc. ?

  2. By NOT replacing, we support more complex content within the md-text-float body:

<md-text-float ng-model="myValue">
  <span class="employee">{{myLabel}}</span> rocks!
</md-text-float>

So to combine both ideas:

<md-text-float ng-model="myValue" ng-blur="doSomethingOnBlur()" autocapitalize="off">
  <span class="employee">{{myLabel}}</span> rocks!
</md-text-float>

becomes

<md-input-group>
   <label for="">
      <span class="employee">{{myLabel}}</span> rocks!
   </label>
   <input ng-model="myValue" ng-blur="doSomethingOnBlur()" autocapitalize="off" type="">
   <md-ripple-container></md-ripple-container>
</md-input-group

@ajoslin
Copy link
Contributor Author

ajoslin commented Nov 6, 2014

I remember us talking about this before. And you're right, we decided we'd have to copy Angular's internal implementation of "copy attributes to children". We should definitely do that, then.

@marcysutton
Copy link
Contributor

As long as the label and input have a valid for/id pairing, the above code should be fine. I wish there was a way to make it work with a native input element, but at least this way we can ensure there will be a valid label for every text input.

@NitsanBaleli
Copy link

I wonder are the above suggestions considered a fix? or are we waiting for 0.7.0 version to fix this issue?

@gkalpak
Copy link
Member

gkalpak commented Dec 4, 2014

@NitsanBaleli: I believe we are waiting for a fix :)

I've left a comment that seems very relevant to this issue as well at #848 (comment).

A couple of thoughts:

  1. Using replace is not a good idea. It is deprecated and its use is discouraged.
    The main problem is not the deprecation itself (since it might never get actually removed), but the reason it got deprecated, namely the many issues and corner cases that arise from its use.
  2. Copying attributes seems to be the way to go, but there are cases where the user will need some attributes to stay on the original element and some to be copied to the generated input. (E.g. an ngRepeat or class attribute might need to stay on the original element, while ngBlur copied to the input.) So, there should be a way to let the user differentiate between the two categories.

@shprink
Copy link
Contributor

shprink commented Dec 5, 2014

Can't wait for this feature :)

@luckyadam
Copy link

I think you can use like this

<md-input-group class="md-default-theme">
   <label for="">something </label>
   <md-input  ng-model="myValue" ng-change="doSomethingOnChange()"  ng-blur="doSomethingOnBlur()" autocapitalize="off" type=""></md-input>
</md-input-group>

@NitsanBaleli
Copy link

@luckyadam so far so good, thank you

@epelc
Copy link
Contributor

epelc commented Dec 11, 2014

We may be able to do something like bootstrap where you use the regular html5 elements label and input and perhaps an outer div with a class on it to handle the styling. This would fix all the problems people are currently having with attributes not being passed down to the inner input.

It might look something like this

<div class="input-group">
    <!-- add an option "float" class to have floating labels like the current
    <md-text-float> otherwise the label would appear above the input -->
    <label>Email</label>
    <!-- placeholder would not be required if the above label has the "float" class -->
    <input type="email" placeholder="Enter your email address">
</div>

This would probably add extra html to most places but it seems like a good way to fix the current attribute problem. Also maybe the outer div could be replaced with something like md-input so we wouldn't need to specify the input-group class.

@ghost
Copy link

ghost commented Dec 26, 2014

THIS WORK FOR ME:

<md-input-group> 
   <label for="message" class="text-muted">Enter message...</label> 
   <md-input id="message" 
        ng-model="message" 
        name="message" 
        id="message" 
        ng-keypress="typing($event, user.inroom)" on-focus="focus(true)" 
        on-blur="focus(false)"> 
   </md-input> 
</md-input-group>

@ilanbiala
Copy link
Contributor

So is there a fix being implemented, or are we left to use a workaround for now?

@demisx
Copy link

demisx commented Jan 4, 2015

+1

@shprink
Copy link
Contributor

shprink commented Jan 8, 2015

Thanks @ajoslin !

Splaktar added a commit to Splaktar/material that referenced this issue Jan 13, 2015
@marcospgp
Copy link

So did you give up on the idea of copying the attributes? I think it would be much better to have a single line for an input rather than 4. You guys seemed to be talking about working around the errors and then a commit that just gives up on that idea comes up? I'm a little confused

@marcospgp
Copy link

@gkalpak commented this "Copying attributes seems to be the way to go, but there are cases where the user will need some attributes to stay on the original element and some to be copied to the generated input. (E.g. an ngRepeat or class attribute might need to stay on the original element, while ngBlur copied to the input.) So, there should be a way to let the user differentiate between the two categories."

That makes a lot of sense and seems like a valid reason to keep the inputs as they are right now. The one thing you could do is add this explanation to the docs, so people like me who go there and wonder why they have to add 4 lines for a directive can at least know why that way is better 😄

@epelc
Copy link
Contributor

epelc commented Jan 18, 2015

@marcosportugal there are so many edge cases with copying the attributes. It makes it really hard to have good for validation and it further completes working with any third party directives.

I think the container element is a good solution it's fixed most of my problems with form validation and it makes me wish checkboxes and other elements worked this way.

@demisx
Copy link

demisx commented Jan 18, 2015

I really like the current container approach. Though, more lines, but makes so many things straightforward. 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests