Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for inline paper-icon in paper-input component. #154

Merged
merged 1 commit into from
Aug 19, 2015
Merged

Add support for inline paper-icon in paper-input component. #154

merged 1 commit into from
Aug 19, 2015

Conversation

mike1o1
Copy link
Contributor

@mike1o1 mike1o1 commented Aug 18, 2015

Also add support for placeholder text, and add option to hide all error messages. If no label value, don't render the label element, so that user can opt out of floating labels by using placeholders instead.

Also add support for placeholder text, and add option to hide all error messages. If no label value, don't render the label element, so that user can opt out of floating labels by using placeholders instead.
miguelcobain added a commit that referenced this pull request Aug 19, 2015
Add support for inline paper-icon in paper-input component.
@miguelcobain miguelcobain merged commit 598606a into miguelcobain:master Aug 19, 2015
@jamesdixon
Copy link

@mike1o1 had a question on the addition of placeholders. This works great, but say for example I also wanted to have both a label and a placeholder for a field like an SSN. The label may say 'SSN' and the placeholder would be something like a hint as to the format (xxx-xx-xxxx). At the moment, I can't have both a label and placeholder as they overlap each other. Ideally, when the form is out of focus, just the label would display as it normally does and then when focused, the label would transition and the placeholder would display.

I just attempted to implement this and was successful, but I'm not sure the method for doing so is the ember-paper-way as I'm new to the library and admittedly, I have zero experience with SASS. Here's my current implementation:

// addon/components/paper-input.js
actions: {
    focusIn(value) {

      if(this.get('placeholder') && this.get('label')) {
        this.$('#' + this.get('inputElementId')).attr('placeholder', this.get('placeholder'));
      }
      // We resend action so other components can take use of the actions also ( if they want ).
      // Actions must be sent before focusing.
      this.sendAction('focus-in', value);
      this.set('focus',true);

    },
    focusOut(value) {
      if(this.get('placeholder') && this.get('label')) {
        this.$('#' + this.get('inputElementId')).attr('placeholder', '');
      }

      this.sendAction('focus-out', value);
      this.set('focus',false);
      this.set('isTouched', true);
    },
    keyDown(value, event) {
      this.sendAction('key-down', value, event);
    }
  }

Is there a better way to implement this that's more in-line with ember-paper conventions?

Thanks in advance,
James

@jamesdixon
Copy link

Just a follow-up. I believe I figured out the right way to do it. It appears there was SASS in paper-input.css for .md-placeholder, but it appears to be unused. Any idea why that may be?

@jamesdixon
Copy link

@mike1o1 here's what I ended up with to allow labels and placeholders to co-exist: https://github.com/jamesdixon/ember-paper/tree/feature/enhanced-placeholder

The only thing I'm a bit unsure of is how the placeholder is positioned. The original positioning of that element wasn't correct, so I adjusted as best as I could, but not sure it's the best way of doing it. Would greatly appreciate your input!

Best,
James

@mike1o1
Copy link
Contributor Author

mike1o1 commented Aug 23, 2015

The current implementation, as well as the angular material implementation that we're taking our stylesheets and inspiration from, does not support both a placeholder and a label. A placeholder is only used if there is no label set. Ember-paper requires you to be more explicit, and if no label is set, then there is no floating label at all, whereas Angular will convert your placeholder to a floating label. Angular material throws a warning if you use both.

https://github.com/angular/material/blob/6f31200501db39903253f95c632494a734e6720d/src/components/input/input.js#L406

What I think you're looking for is more of a input mask, rather than a placeholder isn't it? A placeholder usually goes away as soon as a user inputs some text.

Not sure if this helps, but I'd look into using an input mask instead of using a placeholder.

@jamesdixon
Copy link

@mike1o1 you're right: for the example I mentioned, an input mask is more appropriate. What I was actually thinking of, and wasn't conveyed with the example, is something more akin to hints. For example, imagine you had a password field and you have a label, "Password," you may also want to specify that the password must be over 8 characters. All that said, even with the new example, a placeholder isn't the best place for a hint; it belongs outside the field.

Sorry for bothering you in this. Glad I was at least able to jump into the code and understand it a bit better!

On another note, are you guys planning on strictly adhering to the Angular Material implementation? While everything is according to spec, I think it's safe to say they haven't covered every situation.

Thanks!
James

@mike1o1
Copy link
Contributor Author

mike1o1 commented Aug 23, 2015

It's no bother at all! I'm glad you were able to get an understanding of the code as well. I hope my comment didn't come off as discouraging the use of using the two side by side, it's just that it's not something I think we'll support out of the box. Another option is to extend the component and take your previous implementation.

I agree with the hints and that your examples would be better suited using them. That's something that is in the material spec on the errors page, and something we support with validation messages in the $warn color, but seems we don't support regular hint messages (and neither does angular material).

I have an idea on how to implement something, but it'll take some experimenting. For now, it would probably be a good idea to create a new issue for supporting helper text and we'll see what we can do.

Thanks!
Mike

@jamesdixon
Copy link

@mike1o1 not discouraging whatsoever! I became so caught up in getting it working that I completely missed that the user experience didn't make sense. Appreciate you bringing me back to earth on that one!

@mike1o1 mike1o1 deleted the paper-input-icon-support branch October 16, 2015 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants