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
[Chrome] Add preventScroll option. #3186
Conversation
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.
A couple of nitpicks here, but otherwise this looks really good. Thank you, @jpmedley!
api/HTMLElement.json
Outdated
} | ||
}, | ||
"status": { | ||
"experimental": false, | ||
"standard_track": true, | ||
"deprecated": false | ||
} | ||
}, | ||
"options": { |
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.
Maybe give this a more specific name, given the description? Maybe preventScroll_option
? I couldn't find another example of this sort of thing in the data, so I don't think we have a convention to adhere to.
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.
Looking at this with fresh eyes, I realized this is similar to init objects on constructors. I now think that neither is the right approach. We probably need a structure with a generic options/init and its actual options as children. This would give us the flexibility to programmatically how this info is displayed. For example, we could make the list of options expandable/collapsable.
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.
preventScroll_option would be consistent with what we have elsewhere so far. C.f.:
browser-compat-data/api/AudioContext.json
Line 152 in f4f8ce8
"latencyHint": { browser-compat-data/api/Clients.json
Line 224 in f4f8ce8
"includeUncontrolled_option": {
I agree it is not perfect, but it works for now. I think coming up with additional structure should be discussed separately.
Co-Authored-By: jpmedley <jmedley@google.com>
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.
Thank you!
https://www.chromestatus.com/features/5745122025144320