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

Change directions to angles in linearGradient. Otherwise the CSS com… #222

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ddssff
Copy link
Contributor

@ddssff ddssff commented Jan 2, 2022

…es out

"linear-gradient(bottom, ..." rather than "linear-graident(to bottom, ...".
With this we get "linear-gradient(180deg, ...".

…s out

"linear-gradient(bottom, ..." rather than "linear-graident(to bottom, ...".
With this we get "linear-gradient(180deg, ...".
@turion
Copy link
Collaborator

turion commented Jan 7, 2022

I think linearGradient and repeatingLinearGradient are the only functions using this Direction. Which means:

  • repeatingLinearGradient has the same bug like linearGradient
  • we could just change the Val instance to your fix instead, essentially fixing both functions

Looking into https://www.w3schools.com/csSref/func_repeating-linear-gradient.asp, I'm actually getting the impression that Direction is not defined in a sensible way to begin with. A better definition would be something like:

data Direction = DirectionSide Side | DirectionCorner Side side | DirectionAngle (Angle Deg)

instance Val Direction where
  value (DirectionSide side) = "to " <> value side
  ...

(An even better definition would allow any kind of angle, not just degrees.)

But I can understand if redefining Direction like that is too much for a quick fix PR.

@ddssff
Copy link
Contributor Author

ddssff commented Jan 10, 2022

Maybe we could write new tickets for further improvements.

@turion
Copy link
Collaborator

turion commented Jan 11, 2022

True, then the best path forward is changing the Val instance, I think.

@turion
Copy link
Collaborator

turion commented Jul 7, 2023

Can you add a unit test for this?

@ddssff
Copy link
Contributor Author

ddssff commented Jul 7, 2023

uh oh I don't even remember what a linear gradient is 🙃

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

Successfully merging this pull request may close these issues.

2 participants