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(text-field): Adjust the baseline of text field's helper text to match spec #3069

Merged
merged 20 commits into from
Aug 1, 2018

Conversation

abhiomkar
Copy link
Collaborator

@abhiomkar abhiomkar commented Jul 13, 2018

Fixes #2783

BREAKING CHANGE

Changes included:

  • Text field's helper text block element uses typography baseline mixin to set baseline height.
  • Removed margin bottom from both text field root and helper text.
  • Updated the text field demos.
  • Changes to typography baseline mixin to use flexbox to avoid extra white-space on newline.

@codecov-io
Copy link

codecov-io commented Jul 13, 2018

Codecov Report

Merging #3069 into master will increase coverage by 0.11%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3069      +/-   ##
==========================================
+ Coverage   98.24%   98.35%   +0.11%     
==========================================
  Files         120      120              
  Lines        5129     5118      -11     
  Branches      638      638              
==========================================
- Hits         5039     5034       -5     
+ Misses         90       84       -6
Impacted Files Coverage Δ
packages/mdc-select/index.js 90.1% <0%> (-0.98%) ⬇️
packages/mdc-textfield/index.js 93.08% <0%> (-0.09%) ⬇️
packages/mdc-textfield/foundation.js 98.92% <0%> (-0.02%) ⬇️
packages/mdc-snackbar/index.js 100% <0%> (ø) ⬆️
packages/mdc-dialog/index.js 100% <0%> (ø) ⬆️
packages/mdc-dialog/constants.js 100% <0%> (ø) ⬆️
packages/mdc-select/constants.js 100% <0%> (ø) ⬆️
packages/mdc-select/foundation.js 100% <0%> (ø) ⬆️
packages/mdc-dialog/foundation.js 100% <0%> (ø) ⬆️
packages/mdc-textfield/constants.js 100% <0%> (ø) ⬆️
... and 1 more

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 e161cc0...3d304b4. Read the comment docs.

@abhiomkar abhiomkar changed the title Adjust the baseline of text field's helper text to match spec fix(text-field): Adjust the baseline of text field's helper text to match spec Jul 16, 2018
@abhiomkar abhiomkar changed the base branch from master to typography_text_baseline July 16, 2018 15:01
@abhiomkar abhiomkar changed the base branch from typography_text_baseline to master July 25, 2018 16:49
@mdc-web-bot
Copy link
Collaborator

🤖 Beep boop!

Screenshot test report

Commit b132601 vs. master:

No diffs! 💯🎉

aria-hidden="true" style="display:none;">
Helper Text (possibly validation message)
aria-hidden="true" style="display:none;"
>Helper Text (possibly validation message)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the helper text sensitive to leading whitespace now? AFAICT it was not previously?

If this is a limitation of the new baseline styles/mixin, I'd like to see us work it out if at all possible...

Copy link
Contributor

@kfranqueiro kfranqueiro left a comment

Choose a reason for hiding this comment

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

One potential cleanup item, otherwise LGTM (don't forget BREAKING CHANGE: ... in the actual squash commit description)

<p id="username-helper-text" class="mdc-text-field-helper-text" aria-hidden="true">
This will be displayed on your public profile
</p>
<p id="username-helper-text" class="mdc-text-field-helper-text" aria-hidden="true">This will be displayed on your public profile</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this formatting change can be reverted?

@abhiomkar abhiomkar merged commit 36acc28 into master Aug 1, 2018
@abhiomkar abhiomkar deleted the textfield_helper_text_baseline_issue2783 branch August 2, 2018 15:07
@jamesmfriedman jamesmfriedman mentioned this pull request Aug 23, 2018
48 tasks
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.

4 participants