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

fix(select): Fix pristine/dirty error, scope conflict, and many styles. #8672

Conversation

topherfangio
Copy link
Contributor

@topherfangio topherfangio commented Jun 6, 2016

Fix styles and code to follow pristine/dirty styling of other input elements and provide CSS class for stand-alone usage.

  • Select now behaves like a normal input, appearing as invalid
    if the user focuses/blurs the element, or submits the form,
    without selecting an option.
  • Fix issues with floating labels not working on focus.
  • Add new md-no-underline CSS class to allow for stand-alone
    usage (non-form).
  • Update demos to show new stand-alone usage with required
    example.
  • Standardize asterisk visibility when required when standalone
    or inside of a <md-input-container>

Additionally, the select component currently sets the isOpen variable on the $scope. This can cause conflicts if the user has their own isOpen variable on the scope.

Fix by privatizing our own variable to _mdSelectIsOpen to reduce chances of a conflict.

Fixes #8529. Fixes #7988. Fixes #8527.

@topherfangio topherfangio added needs: review This PR is waiting on review from the team needs: manual testing This issue or PR needs to have some manual testing and verification done labels Jun 6, 2016
@topherfangio
Copy link
Contributor Author

topherfangio commented Jun 6, 2016

@ThomasBurleson @EladBezalel @devversion @crisbeto Can you guys all review this and actually test out the new select demos? Ideally, this would be on a mix of Windows/Mac/iOS/Android/etc.

Edit: To clarify, I have tested on Mac Chrome/Safari/Firefox but would like some additional testing for other devices as well as your opinions.

@crisbeto
Copy link
Member

crisbeto commented Jun 6, 2016

@topherfangio In general LGTM on Chrome, Edge, Firefox and iOS Safari, however it seems kind of trippy on IE11. Here's what I mean:

a

Note the dropdown in the lower right. It doesn't seem to happen in HEAD.

var parent = $mdUtil.getClosest(element, 'form');
var form = parent ? angular.element(parent).controller('form') : null;

return form ? form.$submitted : false;
Copy link
Member

Choose a reason for hiding this comment

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

i would get rid of form and just do:

return parent ? angular.element(parent).controller('form').$submitted : false;

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 actually started with that, but you may not always have a from controller (since this can be used outside of a form) in which case that would throw a NPE. Thus, why I also have a test for this situation 😉

@topherfangio topherfangio added needs: work and removed needs: review This PR is waiting on review from the team labels Jun 6, 2016
@topherfangio
Copy link
Contributor Author

@crisbeto Thanks! I'll see if I can snag an IE11 machine around here somewhere (I think my father-in-law has one) and do some additional testing. Glad you caught that before it went into master 😄

Fix styles and code to follow pristine/dirty styling of other
input elements and provide CSS class for stand-alone usage.

 - Select now behaves like a normal input, appearing as invalid
   if the user focuses/blurs the element, or submits the form,
   without selecting an option.
 - Fix issues with floating labels not working on focus.
 - Add new `md-no-underline` CSS class to allow for stand-alone
   usage (non-form).
 - Update demos to show new stand-alone usage with required
   example.
 - Standardize asterisk visibility when required when standalone
   or inside of a `<md-input-container>`

Additionally, the select component currently sets the `isOpen`
variable on the `$scope`. This can cause conflicts if the user
has their own `isOpen` variable on the scope.

Fix by privatizing our own variable to `_mdSelectIsOpen` to
reduce chances of a conflict.

Fixes angular#8529. Fixes angular#7988. Fixes angular#8527.
@topherfangio topherfangio force-pushed the fix-8529-select-pristine-error branch from 8691063 to bc800e2 Compare June 9, 2016 17:17
@topherfangio topherfangio added needs: review This PR is waiting on review from the team and removed needs: manual testing This issue or PR needs to have some manual testing and verification done needs: work labels Jun 9, 2016
@topherfangio
Copy link
Contributor Author

@ThomasBurleson I believe this is ready for review/merge now. Let me know if it needs any changes.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants