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

Phantom types: background-size #421

Merged

Conversation

BrianHicks
Copy link
Collaborator

This adds backgroundSize and backgroundSize2 for #392. Commits for this start at 48733cd, and I'm happy to rebase onto phantom-types when/if #419 is merged.

I was going to add backgroundSizes to go along with backgroundImages, but I can't figure out a good way of doing the types. Essentially, we need to be able to have all these properties:

/* valid */
background-size: cover;
background-size: 100px;
background-size: 100px, cover;
background-size: 100px 20px, cover;

/* invalid */
backgrouns-size: cover cover;
background-size: 100px 20px, cover cover;

  • Each Value is an open record with a single field. The field's name is the value's name, and its type is Supported. For example foo : Value { provides | foo : Supported }
  • Each function returning Style accepts a closed record of Supported fields.
  • If a function returning Style takes a single Value, that Value should always support inherit, initial, and unset because all CSS properties support those three values!
  • If a function returning Style takes more than one Value, however, then none of its arguments should support inherit, initial, or unset, because these can never be used in conjunction with other values!
  • Every exposed value has documentation which includes at least 1 code sample.
  • Documentation links to to a CSS Tricks article if available, and MDN if not.
  • Make a pull request against the phantom-types branch, not master!

Copy link
Owner

@rtfeldman rtfeldman left a comment

Choose a reason for hiding this comment

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

Just 2 quick documentation requests, and 👍 to rebasing on phantom-types now that #419 is merged!

, vw : Supported
, vmin : Supported
, vmax : Supported
, auto : Supported
Copy link
Owner

Choose a reason for hiding this comment

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

Nice, thanks for not including inherit et al 😄

src/Css.elm Outdated
AppendProperty ("background-size:" ++ width ++ " " ++ height)


{-| Used in [`backgroundSize`](#backgroundSize) to always show the whole
Copy link
Owner

Choose a reason for hiding this comment

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

Can we get CSS Tricks (or, failing that, MDN) links for contain and cover?

@BrianHicks BrianHicks force-pushed the phantom-types--background-size branch from afdf59e to ef8b3a2 Compare May 2, 2018 14:10
@BrianHicks
Copy link
Collaborator Author

done. Ready for another review 😄

@rtfeldman
Copy link
Owner

Fantastic, thank you @BrianHicks!

@rtfeldman rtfeldman merged commit f2ed772 into rtfeldman:phantom-types May 2, 2018
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