fix(styling) compilation bug due to missing calc in variables.scss

[fheck]

I'm submitting a Bug report

Your Environment

Software	Version(s)
Angular	8.2.3
Angular-Slickgrid	2.19.0
TypeScript	3.9.6

Describe the Bug

When using a $font-size-base value set to rem, an error occurred during compilation stating "Incompatible units px and rem".

Steps to Reproduce

In styles.scss, before importing the variables.scss, set $font-size-base to 1rem.

Expected Behavior

It compiles.

Current Behavior

It does not compile, with the error mentioned above.

Possible Solution

Add missing calc in lines 276 and 315 as seen in this PR.

Code Sample

Not needed.

[codecov]

Codecov Report

Merging #529 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff             @@
##            master      #529    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          148       148            
  Lines         9634      9634            
  Branches      3227      3366   +139     
==========================================
  Hits          9634      9634            

---

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 36f958d...85f2d61. Read the comment docs.

[ghiscoding]

@fheck
Good catch, however I see a few more missing the calc, could you please add it to every other variable, I counted 12 to change in total, you can see this commit to apply your patch in my other lib Slickgrid-Universal

[fheck]

I added all I could find, with a total of 12 it matches yours. Initially I only fixed those two, because that were the only ones that prevented compilation for me and I didn't look beyond.

[ghiscoding]

Thanks for that, I'll release a new version in a week or 2 so hopefully you can wait a bit more

[fheck]

Yeah, I've rolled back the upgrade for now because the actual problem I was looking into is somewhere with Slickgrid itself and the update did not fix it yet. So I need to look into it some more anyways.

[ghiscoding]

what kind of problem and/or upgrade have you done? do you mean some issues in your code or the lib?

[fheck]

I believe it is in the slickgrid lib. I can't tell tell the exact problem yet, but when having frozen columns, searchable columns and autoHeight enabled, slickgrid calculates the height of the viewport too small by the height of the header row (the one with the search boxes). Currently I do not have a minimal example because we have an additional wrapper layer on top of the your module (for embedding with an existing data source).
I had hoped your PRs in the slickgrid code would help but even with the current version the problem persists. I will look into it some more..

[ghiscoding]

I personally never use the autoHeight flag as I prefer to only have auto-resizable grid or fixed sizes, but the last time we touched the autoHeight in the SlickGrid core lib was this issue which was also reported in this Angular-Slickgrid issue... not sure if that has anything to do with your issue but you might want to read and/or try some changes in the core lib. I know there was an issue once in the core and we introduced a bug while trying to fix something, we ended up having to revert it since it introduced a bug (anyway you can read the full story in the issues I referenced).

[ghiscoding]

@fheck
This PR is actually causing me some problems in my other lib, SASS doesn't like to calc() with SASS variables and it has to be interpolate before trying to use a SASS variables. As answered in this SO - Answer

Basically this
calc(17px * $header-row-count)
should be changed to this (notice the #{sass-var}
calc(17px * #{$header-row-count})

I'd be happy to confirm that this also works on your side.

It sadly took me about 2 hours to find the issue, kinda hard when there's no console errors displayed. I guess I need to add more Cypress tests to cover that, for reference here's my commit in my other lib.

EDIT

Fixed in #536

[fheck]

Can confirm, that works for me as well. Sorry I missed that.

[ghiscoding]

no worries and thanks for the confirmation, I've pushed a new version 2.19.1 including this PR.

Thanks for the contribution and please upvote ⭐ if you like the lib (if that is not already done)
Cheers

[ghiscoding]

@fheck
This is now available in the new release 2.20.1

Cheers ⭐