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(textfield): Fix textarea label from overlapping border. #1715

Merged
merged 13 commits into from
Jan 10, 2018

Conversation

moog16
Copy link
Contributor

@moog16 moog16 commented Dec 7, 2017

Removed textarea border to prevent double borders.
Added line-height to textarea label.

Fixes #1608
https://www.pivotaltracker.com/story/show/153034989

Removed textarea border to prevent double borders.
@codecov-io
Copy link

codecov-io commented Dec 7, 2017

Codecov Report

Merging #1715 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1715   +/-   ##
=======================================
  Coverage   99.43%   99.43%           
=======================================
  Files          84       84           
  Lines        3721     3721           
  Branches      484      484           
=======================================
  Hits         3700     3700           
  Misses         21       21

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae1993a...e1fab5d. Read the comment docs.

@@ -445,7 +445,6 @@
.mdc-text-field__input {
padding: $padding-inset;
padding-top: $padding-inset * 2;
border: 1px solid transparent;
Copy link

@havenchyk havenchyk Dec 8, 2017

Choose a reason for hiding this comment

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

I'm not 100% sure, but in active state (:focus) textarea should have 2px border (now it's a sum of 1px div border and 1px textarea border) and if you just remove this border from textarea, the whole border will become 1px

screencast 2017-12-08 15-39-06

@havenchyk
Copy link

@moog16 can transparent background solve the initial problem instead of line-height? https://github.com/material-components/material-components-web/blob/master/packages/mdc-textfield/mdc-text-field.scss#L479

@kfranqueiro
Copy link
Contributor

I believe the reason for the non-transparent background is so that the label is still readable in cases where the text entered into the textarea overlaps it due to overflow/scrolling.

@amsheehan amsheehan self-assigned this Dec 11, 2017
@moog16
Copy link
Contributor Author

moog16 commented Dec 12, 2017

So a few things are being addressed in this PR:

  1. There was another issue when moving the fullwidth textarea handle horizontally, and the outer element would not collapse/expand with the textarea element. I fixed this with the resize; vertical; property.
  2. The 2nd issue would also come up when it was not fullwidth. The textarea wrapper element needed the css property width: fit-content, which allows for the outer div to take the width of the children.
  3. I also added line-height to the label, which was also used in normalize.css. There is a conflict with Safari's default line-height that caused the overlap in the top border.

Copy link
Contributor

@amsheehan amsheehan left a comment

Choose a reason for hiding this comment

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

LGTM

@moog16 moog16 merged commit 673a84d into master Jan 10, 2018
@moog16 moog16 deleted the fix/textarea/label-overlap branch January 10, 2018 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants