Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

fix(text-field): Input position and textarea size #3321

Merged
merged 9 commits into from
Aug 13, 2018

Conversation

acdvorak
Copy link
Contributor

@acdvorak acdvorak commented Aug 9, 2018

BREAKING CHANGE: The mdc-text-field--upgraded class has been removed. mdc-text-field__input position has changed by 2px to match spec. mdc-text-field--textarea width in IE and Edge now matches other browsers.

What it does

  • Fixes vertical text alignment of mdc-text-field__input in the mdc-text-field--box variant to match spec. Baseline is now 28px instead of 30px. See mdc-textfield Outlined incorrect specs #2826.
  • Fixes textarea width in IE and Edge:
    • Replaces unsupported values fit-content and initial with auto
  • Removes the mdc-text-field--upgraded class:
    • Prevents text fields from resizing and causing browser reflow jank after the JS initializes
    • Most properties from mdc-text-field--upgraded have been moved to mdc-text-field
    • Sets display: inline-flex, box-sizing: border-box, and margin-top: 16px on all variants

Example output

Before:

before

After:

after

Spec:

Text field guidelines

image

BREAKING CHANGE: The `mdc-text-field--upgraded` class has been removed. It was a holdover from CSS-only text-fields.

- Moves most properties from `mdc-text-field--upgraded` to `mdc-text-field`
    - Sets `display: inline-flex`, `box-sizing: border-box`, and `margin-top: 16px` on all text fields
    - Prevents text fields from resizing and triggering reflow after the JS initializes
- Removes the `mdc-text-field--upgraded` class
- Fixes vertical text alignment of `mdc-text-field__input` in the `mdc-text-field--box` variant to match spec. Baseline is now 28px instead of 30px.
@acdvorak
Copy link
Contributor Author

acdvorak commented Aug 9, 2018

Wait for #3306 to be merged before merging this PR.

@kfranqueiro
Copy link
Contributor

Might also want to wait for #2859 which will almost undoubtedly conflict with this?

@codecov-io
Copy link

codecov-io commented Aug 9, 2018

Codecov Report

Merging #3321 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3321      +/-   ##
==========================================
- Coverage   98.05%   98.05%   -0.01%     
==========================================
  Files         120      120              
  Lines        5143     5141       -2     
  Branches      643      643              
==========================================
- Hits         5043     5041       -2     
  Misses        100      100
Impacted Files Coverage Δ
packages/mdc-textfield/foundation.js 98.93% <ø> (-0.02%) ⬇️
packages/mdc-textfield/constants.js 100% <ø> (ø) ⬆️

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 2b15026...dea987b. Read the comment docs.

@acdvorak acdvorak changed the title fix(text-field): Prevent jank by removing --upgraded fix(text-field): Prevent FOUC jank by removing --upgraded Aug 9, 2018
@acdvorak acdvorak changed the title fix(text-field): Prevent FOUC jank by removing --upgraded fix(text-field): Remove --upgraded class to avoid jank Aug 12, 2018
@mdc-web-bot
Copy link
Collaborator

🤖 Beep boop!

Screenshot test report 🚦

152 screenshots changed from master on commit 8bd3c56:

Details

152 Changed:

@acdvorak acdvorak changed the title fix(text-field): Remove --upgraded class to avoid jank fix(text-field): Input position and textarea size Aug 13, 2018
@mdc-web-bot
Copy link
Collaborator

🤖 Beep boop!

Screenshot test report 🚦

152 screenshots changed from master on commit dea987b:

Details

152 Changed:

Copy link
Contributor

@williamernest williamernest left a comment

Choose a reason for hiding this comment

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

LGTM. Good catch on fit-content/initial.

@acdvorak acdvorak merged commit 5160241 into master Aug 13, 2018
@acdvorak acdvorak deleted the fix/text-field/remove-upgraded-jank branch August 13, 2018 22:10
@jamesmfriedman jamesmfriedman mentioned this pull request Aug 23, 2018
48 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants