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

fix(notched-outline): Auto position the notch and floating label based on corner size #3929

Merged
merged 10 commits into from
Oct 18, 2018

Conversation

abhiomkar
Copy link
Collaborator

@abhiomkar abhiomkar commented Oct 17, 2018

This change fixes the issue where notched outline overlaps with floating label on text field & select components.

Notch position is calculated based on the corner size of notched outline and accordingly the shape mixins in text field and select components sets the floating label position based on radius size.

The leading stroke length is calculated with: Math.max(0, numbers.MIN_LEADING_STROKE_EDGE_POSITION - radius - 1.2) which gets shrunk when large radius is used.

The floating label position calculation with: $cornerSize + $leadingStrokeLength + $mdc-notched-outline-notch-gutter-size + 1.2 where $leadingStrokeLength is the stroke length that applies the same formula as above.

Fixes #3158, #3936

@codecov-io
Copy link

codecov-io commented Oct 17, 2018

Codecov Report

Merging #3929 into master will decrease coverage by 0.03%.
The diff coverage is 60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3929      +/-   ##
==========================================
- Coverage   98.52%   98.48%   -0.04%     
==========================================
  Files         120      120              
  Lines        5224     5227       +3     
  Branches      657      657              
==========================================
+ Hits         5147     5148       +1     
- Misses         77       79       +2
Impacted Files Coverage Δ
packages/mdc-notched-outline/constants.js 100% <100%> (ø) ⬆️
packages/mdc-notched-outline/foundation.js 94.87% <50%> (-5.13%) ⬇️

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 e728494...eb4fc3d. Read the comment docs.

@kfranqueiro kfranqueiro force-pushed the textfield_notched_outline branch from 96c0753 to 3a1b4e3 Compare October 17, 2018 14:20
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 since this resolves the bug. After talking to design it looks like we want the notch to move closer to the corner so the label is more inline with the input text and helper text. Filed #3936 to resolve that issue so we can tackle that later.

@abhiomkar
Copy link
Collaborator Author

I updated this PR to not move notch farther when setting large corner size. Now it stays close to corner with a minimum leading notch length. PTAL.

image

@mdc-web-bot
Copy link
Collaborator

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

@riichard
Copy link

riichard commented Nov 9, 2018

I got the following error after upgrading to this version (0.41.0) and using the standard implementation from the readme. The error seems to be related to the changes in this PR. Downgrading to the previous version (0.40.1) resolves the issue.

🚨 Build Error

/my-project-directory/styles.scss:33:25: "12px-mdc-shape-prop-value(small)-1.2" is not a number for `max'

Error: "12px-mdc-shape-prop-value(small)-1.2" is not a number for `max'
        on line 33 of node_modules/@material/notched-outline/_functions.scss, in function `max`
        from line 33 of node_modules/@material/notched-outline/_functions.scss, in function `mdc-notched-outline-get-notch-padded-position`
        from line 249 of node_modules/@material/textfield/_mixins.scss, in mixin `mdc-text-field-outline-shape-radius`
        from line 316 of node_modules/@material/textfield/_mixins.scss, in mixin `mdc-text-field-outlined-`
        from line 116 of node_modules/@material/textfield/mdc-text-field.scss
        from line 8 of stdin
>>   $leadingStrokeLength: max(0, $mdc-notched-outline-min-leading-stroke-edge-
   ------------------------^```

@kfranqueiro
Copy link
Contributor

@riichard your textfield and shape package versions are out of sync. See #4038 (comment).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Notched outline overlaps with text field label
7 participants