mdgriffith / elm-ui

What if you never had to write CSS again?
https://package.elm-lang.org/packages/mdgriffith/elm-ui/latest/
BSD 3-Clause "New" or "Revised" License
1.34k stars 110 forks source link

Element.modular math is wrong (easy fix provided) #331

Open gacallea opened 2 years ago

gacallea commented 2 years ago

Hi and thank you for elm-ui πŸ˜„

Element.modular formula is off because of the use of -1.

Current behavior

Each multiplier returns what the previous should:

Expected behavior

With the above in mind, and with a base, a ratio, and a multiplier:

Versions

Solution

The issue is caused by using the -1 in the function definition. The fix is super easy and saves an else if that's not needed because the negative multiplier suffice:

modular : Float -> Float -> Int -> Float
modular normal ratio rescale =
    if rescale == 0 then
        normal

    else
        normal * ratio ^ toFloat rescale

I put a demo of this on Ellie: https://ellie-app.com/gm2bfCbKdtZa1

Also refer to these to corroborate my findings:

Docs Need Updating too

Contextually, the documentation examples should be fixed/updated as well, to match the corrected formula. Also, to add partial application to the (scaled) examples. Ref. https://github.com/mdgriffith/elm-ui/issues/36

Cheers :)

ps: if needed I would gladly do the PR(s) πŸ’―