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

fluidRange & between don't return the proper calculation when from/toSize don't have the same units as min/maxScreen. #445

Closed
EarthlingDavey opened this issue Jun 5, 2019 · 6 comments
Labels

Comments

@EarthlingDavey
Copy link

  • polished version: 3.4.0
  • JSS-in_CSS library and version: styled-components 4.2.0

Mixin/Helper/Shorthand you were using and how you were using it:

h4 {
    ${fluidRange(
      {
        prop: 'font-size',
        fromSize: 1rem,
        toSize:4rem
      },
      '400px',
      '1000px'
    )}
  }

What you are seeing:

The font-size between screen sizes 400px and 100px is not calculated correctly.

What you expected to see:

A smooth change in font size between 400px and 1000px screen widths

Reproduction:

https://codepen.io/daveybrown/pen/GaLXVE

@bhough
Copy link
Contributor

bhough commented Sep 6, 2019

This will be addressed in v4.

@bhough bhough added bug and removed unconfirmed bug labels Sep 6, 2019
@Grsmto
Copy link

Grsmto commented Nov 2, 2019

Just FYI, there is the same issue with the between function.

@bhough
Copy link
Contributor

bhough commented Nov 5, 2019

@Grsmto Yep, fluidRange is just a wrapper that adds functionality to between.

@kjbrum
Copy link

kjbrum commented Apr 1, 2020

If it's not known already, when using rem units for the fromSize/toSize and the min/max screen sizes, it works correctly.

Updated CodePen: https://codepen.io/kjbrum/pen/yLNrYzv

@bhough bhough changed the title fluidRange breaks with rem units fluidRange & between Do not return the proper calculation when from/toSize don't have the same units as min/maxScreen. May 15, 2020
@bhough bhough changed the title fluidRange & between Do not return the proper calculation when from/toSize don't have the same units as min/maxScreen. fluidRange & between don't return the proper calculation when from/toSize don't have the same units as min/maxScreen. May 15, 2020
@bhough
Copy link
Contributor

bhough commented May 15, 2020

The issue here is not specifically with rem, but is instead with using a different unit of measure for the screen units and the unit you rule you want to be fluid (in this case font-size.)

Ultimately, we have decided to make this an error. Given all the potential cases needed to normalize the two units as well as the decisions we will have to try and make on behalf of the user (which unit to convert to/from, what base to use for em/rem, etc..), we would have to dramatically overload the module to account for all the conversion checks and the conversions themselves.

We do offer some helpers to convert to rem and em for the cases where you might not have control over the units.

bhough added a commit that referenced this issue May 15, 2020
…n/max

Added an error case when the unit for to/fromSize is different than min/maxScreen. This was causing
bad calculations based on the differing scales.

BREAKING CHANGE: If you were using a mix of unit and unitless measure or working with different
units with similar scales (em/rem) you will now get an error where you may have been getting a
seemingly valid calculation.

fix #445
bhough added a commit that referenced this issue May 15, 2020
…max (#511)

* chore(library): update dependencies

Update dependencies and fix new flow error in triangle.test.js

* refactor(stripunit): fully deprecate returnUnit

Fully deprecate returnUnit functionality and refactor return.

* refactor(readablecolor): make strict mode default

Make strict mode default when passing custom colors.

* build(babel): enable bugfix: true

Enable bugfix:true in build and target explicit browser list

* build(lint-staged): remove superfulous git add

* build(lerna): lerna init

* feat(normalize): upgrade to normalize 8.0.1

* build(docs): removed documentation.js auto generation

* feat(fontface): now defaults to looking for local font first

Default is now to look for a local font of the same family-name before downloading. Can be turned
off by passing null to localFonts.

BREAKING CHANGE: localFont will now be populated by default and may have unexpected behavior.

* feat(important): important helper for module rule specificity

important helper for targeting specific rules in module returns for adding `!important`-level
specificity.

* chore(important): update to not mutate original parameter

* chore(important): add to index.js

* fix(triangle): triangle now properly supports inherit

triangle now returns properties in a way that allows inherit to work by declaring an individual side
for the triangle color.

* feat(cssvar): allow a default value for cssVar

Allow the user to pass a default value to cssVar for when a variable is not found.

* fix(between): added error case when to/from unit is different than min/max

Added an error case when the unit for to/fromSize is different than min/maxScreen. This was causing
bad calculations based on the differing scales.

BREAKING CHANGE: If you were using a mix of unit and unitless measure or working with different
units with similar scales (em/rem) you will now get an error where you may have been getting a
seemingly valid calculation.

fix #445
bhough added a commit that referenced this issue Jun 12, 2020
…max (#511)

* chore(library): update dependencies

Update dependencies and fix new flow error in triangle.test.js

* refactor(stripunit): fully deprecate returnUnit

Fully deprecate returnUnit functionality and refactor return.

* refactor(readablecolor): make strict mode default

Make strict mode default when passing custom colors.

* build(babel): enable bugfix: true

Enable bugfix:true in build and target explicit browser list

* build(lint-staged): remove superfulous git add

* build(lerna): lerna init

* feat(normalize): upgrade to normalize 8.0.1

* build(docs): removed documentation.js auto generation

* feat(fontface): now defaults to looking for local font first

Default is now to look for a local font of the same family-name before downloading. Can be turned
off by passing null to localFonts.

BREAKING CHANGE: localFont will now be populated by default and may have unexpected behavior.

* feat(important): important helper for module rule specificity

important helper for targeting specific rules in module returns for adding `!important`-level
specificity.

* chore(important): update to not mutate original parameter

* chore(important): add to index.js

* fix(triangle): triangle now properly supports inherit

triangle now returns properties in a way that allows inherit to work by declaring an individual side
for the triangle color.

* feat(cssvar): allow a default value for cssVar

Allow the user to pass a default value to cssVar for when a variable is not found.

* fix(between): added error case when to/from unit is different than min/max

Added an error case when the unit for to/fromSize is different than min/maxScreen. This was causing
bad calculations based on the differing scales.

BREAKING CHANGE: If you were using a mix of unit and unitless measure or working with different
units with similar scales (em/rem) you will now get an error where you may have been getting a
seemingly valid calculation.

fix #445
bhough added a commit that referenced this issue Sep 16, 2020
…max (#511)

* chore(library): update dependencies

Update dependencies and fix new flow error in triangle.test.js

* refactor(stripunit): fully deprecate returnUnit

Fully deprecate returnUnit functionality and refactor return.

* refactor(readablecolor): make strict mode default

Make strict mode default when passing custom colors.

* build(babel): enable bugfix: true

Enable bugfix:true in build and target explicit browser list

* build(lint-staged): remove superfulous git add

* build(lerna): lerna init

* feat(normalize): upgrade to normalize 8.0.1

* build(docs): removed documentation.js auto generation

* feat(fontface): now defaults to looking for local font first

Default is now to look for a local font of the same family-name before downloading. Can be turned
off by passing null to localFonts.

BREAKING CHANGE: localFont will now be populated by default and may have unexpected behavior.

* feat(important): important helper for module rule specificity

important helper for targeting specific rules in module returns for adding `!important`-level
specificity.

* chore(important): update to not mutate original parameter

* chore(important): add to index.js

* fix(triangle): triangle now properly supports inherit

triangle now returns properties in a way that allows inherit to work by declaring an individual side
for the triangle color.

* feat(cssvar): allow a default value for cssVar

Allow the user to pass a default value to cssVar for when a variable is not found.

* fix(between): added error case when to/from unit is different than min/max

Added an error case when the unit for to/fromSize is different than min/maxScreen. This was causing
bad calculations based on the differing scales.

BREAKING CHANGE: If you were using a mix of unit and unitless measure or working with different
units with similar scales (em/rem) you will now get an error where you may have been getting a
seemingly valid calculation.

fix #445
@bhough
Copy link
Contributor

bhough commented Sep 20, 2020

This has been addressed in v4 which is now in beta and will be released in the coming weeks:

npm install polished@next
yarn add polished@next

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

No branches or pull requests

4 participants