Skip to content

img attrs: srcset, currentSrc, sizes, referrerPolicy, decoding, loading #40

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

Merged
merged 2 commits into from
Dec 11, 2020

Conversation

toastal
Copy link
Contributor

@toastal toastal commented Dec 1, 2020

Prerequisites

  • Before opening a pull request, please check the HTML standard (https://dom.spec.whatwg.org/). If it doesn't appear in this spec, it may be present in the spec for one of the other purescript-web projects. Although MDN is a great resource, it is not a suitable reference for this project.

Description

Adding missing attributes for HTMLImage
https://html.spec.whatwg.org/multipage/embedded-content.html#dom-img-currentsrc

It would be nice to include decode(), but since it returns a Promise and this project doesn't include Aff, I skipped it.

Copy link
Member

@garyb garyb left a comment

Choose a reason for hiding this comment

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

Thanks for adding these, but can we introduce sum types for all the stringy attributes that only actually have a set of acceptable values? CanPlayType in this repo would be an existing example of that.

@toastal
Copy link
Contributor Author

toastal commented Dec 6, 2020

I wouldn't disagree, but it seemed like a lot of the APIs exposed in this repo are just stringly-typed. Can do though.

@toastal
Copy link
Contributor Author

toastal commented Dec 6, 2020

I think referrer-policy may be out of scope as it refers to specs outside of HTML. Not really sure what's best @garyb. Though in going through the ADTs, I caught an error in crossOrigin.

@toastal toastal force-pushed the imageattrs branch 7 times, most recently from fa818b1 to 7ab3f78 Compare December 6, 2020 15:19
Copy link
Contributor

@thomashoneyman thomashoneyman left a comment

Choose a reason for hiding this comment

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

This looks good to me, though I would appreciate another look from @garyb.

"lazy" -> Just Lazy
_ -> Nothing


Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

@garyb
Copy link
Member

garyb commented Dec 10, 2020

All seems reasonable to me! Thanks for adding those sums.

@thomashoneyman thomashoneyman merged commit bb37b29 into purescript-web:master Dec 11, 2020
@toastal toastal deleted the imageattrs branch December 11, 2020 06:24
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.

3 participants