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

Add support for aspect-ratio CSS property. #266

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

Conversation

chungyc
Copy link
Contributor

@chungyc chungyc commented Oct 20, 2024

This adds an aspectRatio function for generating the aspect-ratio CSS property and an AspectRatio type to represent aspect ratios. To make the ratios feel more like Data.Ratio ratios, this defines a % operator on two Integers generating an AspectRatio. So we could do things like this:

aspectRatio (16%9)
aspectRatio auto

To allow for properties such as aspect-ratio: auto 4/3, we use a withFallback function to generate an AspectRatio:

aspectRatio $ auto `withFallback` (16%9)

It is not legal to do something like aspect-ratio: auto auto. While it would have been nice if we could have enforced this with types, this is hard, so it is enforced as a runtime error instead.

The original plan in #259 was based on a type class. This worked correctly, but not nicely, because types could not be inferred completely. For example, if we did

aspectRatio (16%9)

the compiler would not know what type aspectRatio should accept yet. And it can't infer that 16%9 should be Rational or Ratio Int because it doesn't know whether 16 or 9 are Integers or Ints. We could resolve the deadlock by enabling type defaulting, but lots of code disable it (e.g., when turning on -Wall -Werror). So we would often have to do something like

aspectRatio (16%9 :: Rational)
aspectRatio (auto :: Value)
aspectRatio (auto :: Value, 16%9 :: Rational)

which seemed cumbersome, so I gave up on the approach based on a type class.

Generated Haddock documentation can be seen in clay-haddock.tar.gz. The examples in the documentation may seem odd, but they're written this way so that they can correctly be evaluated in case doctest is used one day to test the examples.

Resolves #259.

Copy link
Collaborator

@turion turion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, your PR is a maintainer's dream :) just a few comments.

spec/Clay/GeometrySpec.hs Outdated Show resolved Hide resolved
src/Clay/Geometry.hs Outdated Show resolved Hide resolved
@chungyc chungyc force-pushed the aspect-ratio branch 4 times, most recently from 7e49e15 to f0a3e6e Compare October 21, 2024 23:24
@chungyc chungyc requested a review from turion October 28, 2024 23:40
chungyc and others added 8 commits October 28, 2024 19:47
Co-authored-by: Manuel Bärenz <[email protected]>
There may come a day when `withFallback` should be used with another
type of value in the `Clay.Geometry` module, but until then,
it is overkill to use a type class.
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.

Support aspect-ratio CSS property
2 participants