Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

fix(select): Ensure md-no-asterisk attribute works. #9347

Merged

Conversation

topherfangio
Copy link
Contributor

After a recent change to the asterisk colors, the md-no-asterisk option was no longer working.

  • Update code to add a new .md-no-asterisk CSS class and fix styles.
  • Add appropriate tests.
  • Add md-no-asterisk="false" to demo so users can easily turn it on
    and test the fixed functionality.

Fixes #9339.

After a recent change to the asterisk colors, the `md-no-asterisk`
option was no longer working.

 - Update code to add a new `.md-no-asterisk` CSS class and fix styles.
 - Add appropriate tests.
 - Add `md-no-asterisk="false"` to demo so users can easily turn it on
   and test the fixed functionality.

Fixes angular#9339.
@topherfangio topherfangio added ui: CSS needs: review This PR is waiting on review from the team P1: urgent Urgent issues that should be addressed in the next minor or patch release. labels Aug 18, 2016
@topherfangio topherfangio added this to the 1.1.1 milestone Aug 18, 2016
@@ -74,7 +74,8 @@ angular.module('material.components.select', [
* @param {expression=} md-selected-text Expression to be evaluated that will return a string
* to be displayed as a placeholder in the select input box when it is closed.
* @param {string=} placeholder Placeholder hint text.
* @param md-no-asterisk {boolean=} When set to true, an asterisk will not be appended to the floating label.
* @param md-no-asterisk {boolean=} When set to true, an asterisk will not be appended to the
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this an opt-out? Would an opt-in not be a better approach?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have always pushed for the opt-in approach; however, this has been around since before 1.0 (before I joined the team I think) so I don't think we should change the API at this point.

All I did here was add a note about it only being evaluated once; not being watched.

@ThomasBurleson
Copy link
Contributor

ThomasBurleson commented Aug 22, 2016

This seems so inverted backwards.... wonder how I missed this one!

@ThomasBurleson ThomasBurleson added pr: merge ready This PR is ready for a caretaker to review and removed needs: review This PR is waiting on review from the team labels Aug 24, 2016
@jelbourn jelbourn merged commit f265a0e into angular:master Aug 24, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P1: urgent Urgent issues that should be addressed in the next minor or patch release. pr: merge ready This PR is ready for a caretaker to review ui: CSS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

md-select: md-no-asterisk not working
3 participants