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

Fix MS Edge and Firefox issues #342, #388 #393

Merged
merged 5 commits into from
May 12, 2016
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/components/input/input.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ $md-input-underline-focused-color: md-color($md-primary);
$md-input-underline-disabled-background-image: linear-gradient(to right,
rgba(0,0,0,0.26) 0%, rgba(0,0,0,0.26) 33%, transparent 0%);

/**
* Removes firefox box-shadow on required fields...
Copy link
Member

Choose a reason for hiding this comment

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

Comment can be something like

/** 
 * Undo the red box-shadow glow added by Firefox on invalid inputs.
 * See https://developer.mozilla.org/en-US/docs/Web/CSS/:-moz-ui-invalid
 */

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

*/
:-moz-ui-invalid {
box-shadow: none;
Copy link
Member

Choose a reason for hiding this comment

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

-2 indent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}

/**
* Applies a floating placeholder above the input itself.
Expand All @@ -33,6 +39,15 @@ $md-input-underline-disabled-background-image: linear-gradient(to right,
}
}

/**
* Applies a floating placeholder above the input before chrome's and safari's auto-fill commits.
* Firefox has selector issues for :-webkit-autofill, thats why this mixin is duplicated and is used seperately below
*/
%md-input-placeholder-floating-autofill {
visibility: visible;
padding-bottom: 5px;
transform: translateY(-100%) scale(0.75);
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't follow the reasoning for why you're duplicating these rules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jelbourn that helped to split generated css

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jelbourn what about this?

Copy link
Member

Choose a reason for hiding this comment

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

You can just make this a mixin (rather than a placeholder) and use it in both places rather than duplicating it.

Copy link
Contributor Author

@davidgabrichidze davidgabrichidze May 11, 2016

Choose a reason for hiding this comment

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

@jelbourn I'm not sure that I clearly understand your note. If you suggest to make one mixin, than it means to rollback change, because it WAS one mixin which doesn't work. Why? It generates joined css, something like this: .md-input-placeholder.md-float:not(.md-empty), .md-input-placeholder.md-float.md-focused, input:-webkit-autofill + .md-input-placeholder which is not selected by FF. Why? input:-webkit-autofill css is invalid for FF and it ignores all joined styles.

How to improve? Well, if we split it in two different groups of css input:-webkit-autofill and others, than it works.

Only way i found to create these two groups in rendered css is to make another mixin. That's why it is 'duplicated'. May be there is some other option to force scss compiler render different groups in css when we use 'extend' of mixins?

Copy link
Member

Choose a reason for hiding this comment

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

These definitions are scss placeholders, not mixins, which is the problem. See
http://thesassway.com/intermediate/understanding-placeholder-selectors

If you change to a single mixin, it should be fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, now it's clear :) - will update soon

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


:host {
display: inline-block;
Expand Down Expand Up @@ -143,7 +158,7 @@ $md-input-underline-disabled-background-image: linear-gradient(to right,
// Once the autofill is committed, a change event happen and the regular md-input
// classes take over to fulfill this behaviour.
input:-webkit-autofill + .md-input-placeholder {
@extend %md-input-placeholder-floating;
@extend %md-input-placeholder-floating-autofill;
}

// The underline is what's shown under the input, its prefix and its suffix.
Expand Down
2 changes: 1 addition & 1 deletion src/components/input/input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export class MdInput implements ControlValueAccessor, AfterContentInit, OnChange
@Input() @BooleanFieldValue() floatingPlaceholder: boolean = true;
@Input() hintLabel: string = '';
@Input() id: string = `md-input-${nextUniqueId++}`;
@Input() maxLength: number = -1;
@Input() maxLength: number = null;
@Input() placeholder: string;
@Input() @BooleanFieldValue() required: boolean = false;
@Input() @BooleanFieldValue() spellcheck: boolean = false;
Expand Down