Skip to content

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Aug 5, 2017

Fixes the input not working correctly in high contrast mode by:

  • Using a margin instead of a border to do the spacing inside the input. Windows renders all borders in high contrast mode, even if they're transparent, which caused the input to have a 5px top border.
  • Using a border to render the underline, instead of a background color.
  • Hiding the ripple, because it was causing the underline to blend in with the background.

Also gets rid of an unused method.

Relates to #6257.

@crisbeto crisbeto requested a review from mmalerba as a code owner August 5, 2017 15:11
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Aug 5, 2017
@crisbeto crisbeto force-pushed the input-high-contrast branch from 6092a28 to 892ed03 Compare August 5, 2017 15:52
Fixes the input not working correctly in high contrast mode by:
* Using a margin instead of a border to do the spacing inside the input. Windows renders all borders in high contrast mode, even if they're transparent, which caused the input to have a 5px top border.
* Using a border to render the underline, instead of a background color.
* Hiding the ripple, because it was causing the underline to blend in with the background.

Also gets rid of an unused method.

Relates to angular#6257.
@crisbeto crisbeto force-pushed the input-high-contrast branch from 892ed03 to 2bd4249 Compare August 5, 2017 15:55
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

If you look at the baseline section of the demo app, this seems to have thrown off the baseline alignment for input

@crisbeto
Copy link
Member Author

crisbeto commented Aug 5, 2017

I'm not sure that it really threw anything off. There's ~1px difference (most likely because of the border changes), but if anything it got the input and select to align, in master there's a 1px offset.

Here are a couple of comparisons, the first one is with these changes, the second one is master. It'll probably be easier to compare if you open in new tabs and switch between the two:

1
2

@mmalerba
Copy link
Contributor

mmalerba commented Aug 7, 2017

Don't worry about the select, it's the odd one out and going to change very soon. This change causes the T in Text Input to be misaligned wrt the T in Text 4.

This PR:
this pr
Master:
master

@jelbourn
Copy link
Member

@crisbeto @mmalerba is this worth revisiting?

@crisbeto
Copy link
Member Author

@jelbourn it definitely is, but we haven't been able to find a way to handle this without throwing off the baseline slightly. We may end up having to have a slightly-off baseline in high contrast mode.

@jelbourn
Copy link
Member

I'll leave this to you and @mmalerba, then

@mmalerba
Copy link
Contributor

I'll try playing with this after my form field variant changes and see if those changes help with this at all

@mmalerba
Copy link
Contributor

mmalerba commented Mar 8, 2018

@crisbeto I'm not 100% sure what changed, but I tried converting that border to a margin just now and the baseline seems to be fine (at least on chrome) so may be worth rebasing and checking if this works now

@crisbeto
Copy link
Member Author

crisbeto commented Mar 18, 2018

@mmalerba I did a quick check by converting the border to a margin (I didn't do the other border change that's in this PR) and it still seems like the baseline is being thrown off.

@mmalerba
Copy link
Contributor

mmalerba commented Mar 19, 2018

@crisbeto interesting, I have both versions up side-by-side and I see no difference flipping between them. We're both talking about this border right? https://github.com/angular/material2/blob/master/src/lib/form-field/_form-field-theme.scss#L175

Edit: ah, I don't see any difference in Chrome, but Firefox it still seems to throw it off. Chrome must have changed something because I used to see the difference in Chrome too

@crisbeto
Copy link
Member Author

Here's what I've got. The top one is master and the bottom is only the border being changed to a magin. A couple of things I noticed though:

  1. I'm not sure where that scrollbar on the textarea is coming from.
  2. The screenshots looked identical if I change the margin through the dev tools. They ended up being slightly different if I change it in the source.

1
2

@mmalerba
Copy link
Contributor

That must be the difference, I just changed it in the devtools. I'll see if I can switch it to margin or padding on a different element when I get a chance

@crisbeto
Copy link
Member Author

Closing in favor of #11736.

@crisbeto crisbeto closed this Jun 10, 2018
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants