-
Notifications
You must be signed in to change notification settings - Fork 43
Feat: add loading dom prop #45
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
Feat: add loading dom prop #45
Conversation
src/ReactDOM.res
Outdated
@@ -291,6 +291,8 @@ module Props = { | |||
@optional | |||
list: string, | |||
@optional | |||
loading: string, /* "lazy", "eager" */, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can specify the values exactly at the type level:
loading: [#lazy | #eager],
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I made the change. I had to wrap lazy in quotes because it is a reserved keyword. Also, I used string because I noticed other props that are have a small/finite number of options just use string. (Like method and inputMode)
This PR fixes #43 once the feedback from yawaramin is fixed :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. The other props use string
type because they predate ReScript's feature of polymorphic variants compiling to strings. I think it would make sense to update the prop types where appropriate.
Is that something we want to do on this PR? |
Not necessarily. I +1'd this PR as I don't think it needs to do anything else. My comment was more about future improvements. |
@Kingdutch @yawaramin sorry for the delay. I can't seem to merge this PR though. |
Hi, I'm quite late for the party. |
I would like to add the loading property for Domprops. This allows someone to add the
loading
property on an image for using html-based browser-native lazy loading.