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

fix(input): duplicate placeholders and data bindings aria-label #8291

Closed
wants to merge 1 commit into from

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented May 1, 2016

  • Fixes the element label being duplicated, if it has a data binding in the placeholder attribute. This was caused by Angular re-adding the placeholder attribute, even though it was removed by the Material placeholder directive.
  • Fixes the aria-label getting the raw data binding (e.g. aria-label="{{expression}}"), when it's gets taken from a dynamic placeholder.

Fixes #8251.
Fixes #8377.

@crisbeto crisbeto added the needs: review This PR is waiting on review from the team label May 1, 2016
// expressions in the placeholder to be evaluated.
$mdUtil.nextTick(function() {
$mdAria.expect(element, 'aria-label', element.attr('placeholder'));
}, false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The next tick timeout is not necessary.

    if (!containerCtrl.label) {
      $mdAria.expect(element, 'aria-label', attr.placeholder);
    }

Using the interpolated attribute is working as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought so too, but try looking at the aria-label on the fiddle from the issue: https://codepen.io/leocaseiro/pen/GZYZxr

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh it works fine for me, when I checked out your changes and modified it to attr.placeholder.

Patched: http://codepen.io/DevVersion/pen/reQepZ
HEAD: http://codepen.io/DevVersion/pen/BKGKYp

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I didn't notice that you used the attr object, I'll give it a shot.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, works the same. I'll update the PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, no problem, thanks for giving a shot ;)

@ThomasBurleson
Copy link
Contributor

@crisbeto - please rebase.

@ThomasBurleson ThomasBurleson added the needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved label Jun 3, 2016
* Fixes the element label being duplicated, if it has a data binding in the placeholder attribute. This was caused by Angular re-adding the placeholder attribute, even though it was removed by the Material placeholder directive.
* Fixes the aria-label getting the raw data binding (e.g. `aria-label="{{expression}}"`), when it's gets taken from a dynamic placeholder.

Fixes angular#8251.
Fixes angular#8377.
@crisbeto crisbeto removed the needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved label Jun 3, 2016
@crisbeto
Copy link
Member Author

crisbeto commented Jun 3, 2016

Done @ThomasBurleson

@ThomasBurleson ThomasBurleson added the pr: merge ready This PR is ready for a caretaker to review label Jun 3, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs: review This PR is waiting on review from the team pr: merge ready This PR is ready for a caretaker to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants