feat(input): add asterisk to input directive #6518
Conversation
devversion
force-pushed
the
feat/input-asterisk
branch
from
ab0156a to
44efec8
Compare
devversion
force-pushed
the
feat/input-asterisk
branch
2 times, most recently
from
39a9934 to
b57bbd0
Compare
EladBezalel
added
the
needs: manual testing
|
LGTM |
| @@ -135,6 +135,7 @@ function labelDirective() { | |||
| * @param {string=} placeholder An alternative approach to using aria-label when the label is not | |||
| * PRESENT. The placeholder text is copied to the aria-label attribute. | |||
| * @param md-no-autogrow {boolean=} When present, textareas will not grow automatically. | |||
| * @param md-no-asterisk {boolean=} Stops appending of an asterisk to the label of required inputs | |||
There was a problem hiding this comment.
I'd change this to
When present, asterisk will not be appended to required inputs label
or something like that
|
@devversion the asterisk size is a bit larger than the spec, can you try and make it smaller? |
EladBezalel
added
needs: work
and removed
needs: manual testing
devversion
force-pushed
the
feat/input-asterisk
branch
from
b57bbd0 to
2eee071
Compare
|
@EladBezalel Okay, it's now smaller and still aligned at the top |
|
looks better, the asterisk size shouldn't be changed but that's because the transform so fixing it will be over-kill. Fix other comments and please add/change/ to the demo 1 input to have |
devversion
force-pushed
the
feat/input-asterisk
branch
from
2eee071 to
58a38b4
Compare
|
@EladBezalel - Done |
EladBezalel
added
pr: merge ready
|
From looking at the spec for required fields, does this handle the label text becoming the warning color on blur? |
| @@ -91,6 +91,12 @@ md-input-container { | |||
| bottom: 100%; | |||
| @include rtl(left, 0, auto); | |||
| @include rtl(right, auto, 0); | |||
|
|
|||
| &.md-required:after { | |||
| content: ' *'; | |||
There was a problem hiding this comment.
Does the content reverse for rtl text?
There was a problem hiding this comment.
Asterisk will be aligned in the same direction as the other text, so it's compatible with RTL, LTR.
But it seems that the placeholders are aligned wrong? (See here.
But in general, Yes asterisk will be reversed for RTL text
There was a problem hiding this comment.
@devversion I know this is old, but I thought I would update this since I just looked at it. The labels are properly "aligned" to the right, it's just that the width is not 100% of the parent. I guess we need to use right: 0 in addition to text-align: right here to fix?
There was a problem hiding this comment.
Ah, just looked a bit more; it's actually an issue with the scale(0.75) transform.
Looks like we can fix it with transform-origin: right;.
|
@Emeegeemee - Yes it does - |
|
@devversion From your screenshot that looks like on focus not on blur. Please watch the demo carefully, when the focus is removed from the input in an invalid state the entire label goes red it doesn't go back to the black/grey color. |
|
@Emeegeemee you can see it for yourself on input demo page |
|
@EladBezalel Thanks, from the site it looks to not make the label change red when focus is lost. from the spec: current implementation: |
Fixes #6511