Bulma: Conform to rules of SaSS unit arithmetic, support rem/px conversion and a rem baseline

Created on 9 Mar 2017  路  5Comments  路  Source: jgthms/bulma


About Bulma.

Bug, not a browser bug.

Overview of the problem

This is about the Bulma CSS framework.




I am sure this issue is not a duplicate, but it is related to #502.

Description

In CSS framework, the uses of removeUnit() violate the rules of SaSS unit arithmetic; every use of removeUnit() makes some implicit assumption about the unit of the value being stripped of the unit (usually "rem" but sometimes "px"). The correct approach is instead to convert the value into the desired unit, and then perform arithmetic on the converted, unit-bearing value.

Steps to Reproduce

  1. In sass/utilities/variables.sass, change $size-6 to any value in rem instead of px. npm run deploy bulma, then read the docs. Watch the delete buttons disappear. (See #502).

Expected behavior

I expected any value for $size-6 that is close to the default of 14px to adjust but not destroy delete button styling.

Actual behavior

The arithmetic in the =delete mixin, because of its implicit assumption that $size-6 is in px, results in width and height values that are 0px for all sizes.

Most helpful comment

Hey @robin900 sorry if my intention was not clear.

I try to keep track of all issues and PRs. The way I let people know I'm working on a issue is by assigning it to myself. For #502, I did 21 days ago. This doesn't mean that I already had a fix, but that I was planning on coming up with one.

In this particular case, the calculations around the delete button and the unit conversions were very complicated and were in fact a mistake from my side. I should not have implemented them.

When I had time this weekend to have a look, I realised the best solution was to get rid of those rem/px conversions, and just implement the delete button in a more simple way.

Although I appreciate you raising a PR to fix that mess, the best solution was to remove that mess completely. My work this weekend made your PR irrelevant, something I could not have predicted before I actually started thinking about the solution.

As for maintainers, it's complicated. For example, someone would have merged your PR, which would have been incorrect.

Anyway, thanks for your contributions. I appreciate it.

All 5 comments

I almost have a solution PR ready, but am struggling to implement rem->px and px->rem conversion so that $size-6 does not have to be in px units just to make delete mixin work. I'm trying to avoid using any of the available rem/px mixins out there and thus introducing another dependency. My idea is to add $rem-baseline px value to variables.sass and then have convertUnit() use that if the conversion is rem->px or px->rem.

@jgthms This is ready for review.

@jgthms You know, if you want to fix #502 and a host of other open issues by working on the fix/form-elements branch from March 12-14, that's fine. But it would be nice to let submitters like me know that you're doing it, so that we don't work hard to solve open issues only to discover that you're solving them in another branch.

Also, please take some time to close open issues that have already been solved, or rejected; and do the same with the open PRs. Or, alternately, designate another maintainer to do so. I'd really like to contribute to this fine project, but it can't be an exercise in frustration for me to do so.

Hey @robin900 sorry if my intention was not clear.

I try to keep track of all issues and PRs. The way I let people know I'm working on a issue is by assigning it to myself. For #502, I did 21 days ago. This doesn't mean that I already had a fix, but that I was planning on coming up with one.

In this particular case, the calculations around the delete button and the unit conversions were very complicated and were in fact a mistake from my side. I should not have implemented them.

When I had time this weekend to have a look, I realised the best solution was to get rid of those rem/px conversions, and just implement the delete button in a more simple way.

Although I appreciate you raising a PR to fix that mess, the best solution was to remove that mess completely. My work this weekend made your PR irrelevant, something I could not have predicted before I actually started thinking about the solution.

As for maintainers, it's complicated. For example, someone would have merged your PR, which would have been incorrect.

Anyway, thanks for your contributions. I appreciate it.

@jgthms Don't forget to visit #547 and let them know your "clarified intention".

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jaredreich picture jaredreich  路  3Comments

fundon picture fundon  路  3Comments

guillecro picture guillecro  路  3Comments

scottgrayson picture scottgrayson  路  3Comments

bigZ-again picture bigZ-again  路  3Comments