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

Address TODO under importToString #77

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

Address TODO under importToString #77

wants to merge 6 commits into from

Conversation

wmjoubert
Copy link
Contributor

Implemented it like this since I need it for the project I'm working on. Handles most common cases. Will add a test if you approve. I suspect NameOrUri should be moved to Css.elm and possibly renamed to NameOrUrl with an Url tag instead of Uri to be more inline with the CSS semantics.

@rtfeldman
Copy link
Owner

Love it! Some thoughts:

  • 👍 for changing it to NameOrUrl
  • Would love some tests!
  • Structure shouldn't depend on anything from Css, so defining it there makes sense—but I think having Css import and then re-export both NameOrUrl and its two constructors is a good idea.

Thanks for this! 😄

@wmjoubert
Copy link
Contributor Author

Could not find a way to re-export a union so created name and url functions in CSS instead. If you prefer the tagged type and know how to re-export them please let me know. I'm also not entirely sure how you intended setting things like imports and charset on a Stylesheet - returning a modified record like in the added test at the moment.

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