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

Remove Constrain{Double, Boolean, DOMString, ULong} and {ULong, Double}Range #5975

Closed
teoli2003 opened this issue Jun 14, 2021 · 12 comments · Fixed by #7319
Closed

Remove Constrain{Double, Boolean, DOMString, ULong} and {ULong, Double}Range #5975

teoli2003 opened this issue Jun 14, 2021 · 12 comments · Fixed by #7319
Labels
Content:WebAPI Web API docs

Comments

@teoli2003
Copy link
Contributor

teoli2003 commented Jun 14, 2021

In mdn/browser-compat-data#11000, @vinyldarkscratch is planning to remove the bcd entries for these 4 typedefs.

This is a good thing™.

But there are timely consequences for MDN content: these four pages will immediately (when yari pick up the new bcd npm package) replace their BCD and spec tables by error messages, which will make them look weird.

I think we should wipe them out of MDN content by replacing every mentions of these typedef (most of the time by using the js type they are typedefing). That way, the MDN users will have the type information they will see in their developer tools and their JavaScript.

With a few ripgrep magic, I see the following tasks to be done:

  1. Removal of ConstrainDouble:
  • Delete the ConstrainDouble page (make it a redirect?)
  • Update mentions in MediaTrackConstraints, MediaTrackConstraints.volume, MediaTrackConstraints.frameRate, MediaTrackConstraints.latency, and MediaTrackConstraints.aspectRatio.
  • Update mentions in InputDeviceInfo.getCapabilities.
  1. Removal of ConstrainDOMString:
  • Delete the ConstrainDOMString page (make it a redirect?)
  • Update mentions in MediaTrackConstraints, MediaTrackConstraints.deviceId, MediaTrackConstraints.displaySurface, MediaTrackConstraints.logicalSurface, MediaTrackConstraints.groupId, MediaTrackConstraints.cursor, and MediaTrackConstraints.facingMode.
  • Update mentions in InputDeviceInfo.getCapabilities.
  • Update mentions in Screen Capture API overview page.
  1. Removal of ConstrainBoolean:
  • Delete the ConstrainBoolean page (make it a redirect?)
  • Update mentions in MediaTrackConstraints, MediaTrackConstraints.autoGainControl, MediaTrackConstraints.logicalSurface, MediaTrackConstraints.noiseSuppression, and MediaTrackConstraints.echoCancellation.
  • Update mentions in InputDeviceInfo.getCapabilities.
  1. Removal of DoubleRange:
  • Delete the DoubleRange page (make it a redirect?)

I would add an extra step to prevent them to come back to haunt us in the future:

  1. Add a new taboo-typedef linter rule forbidding mentions of ConstrainDouble, ConstrainDOMString, ConstrainBoolean, DoubleRange words (and maybe other similar typedef that weren't used yet).
@queengooborg
Copy link
Collaborator

Thanks for opening this issue! @saschanaz also has a PR to remove ConstrainULong and ULongRange as well (see mdn/browser-compat-data#10623), so we should account for them as well.

@teoli2003 teoli2003 changed the title Remove Constrain{Double, Boolean, DOMString} and DoubleRange Remove Constrain{Double, Boolean, DOMString, ULong} and {ULong, Double}Range Jun 14, 2021
@hamishwillee
Copy link
Collaborator

There is a discussion on this here https://github.com/mdn/content/discussions/5828

I'm not sure if there is a canonical result from that, but the implication for me was that it is generally OK to delete separate page documentation for dictionaries and that they should be documented in-place instead.

@teoli2003
Copy link
Contributor Author

Thanks for the info!

So:
6. Removal of ConstrainULong:

  • Delete the ConstrainULong page (make it a redirect?)
  • Update mentions in MediaTrackConstraints, MediaTrackConstraints.sampleRate, MediaTrackConstraints.channelCount, MediaTrackConstraints.width, MediaTrackConstraints.sampleSize, and MediaTrackConstraints.height.
  • Update mentions in InputDeviceInfo.getCapabilities.
  1. Removal of ULongRange:
  • Delete the ULongRange page (make it a redirect?)

@teoli2003
Copy link
Contributor Author

There is a discussion on this here #5828

I'm not sure if there is a canonical result from that, but the implication for me was that it is generally OK to delete separate page documentation for dictionaries and that they should be documented in-place instead.

Yes, at least for dictionaries used in one place (which is most, but not all, of them).

@hamishwillee
Copy link
Collaborator

That was my take-away. I've cross linked back to here in the hope that you can get more expert confirmation than I can provide :-). Also, thanks to you and @vinyldarkscratch for letting us know in advance.

@queengooborg
Copy link
Collaborator

To go along with my PR for BCD, I'd love to tackle this one! I've got a couple of questions before beginning, however:

  • Should we define these constraint typedefs in the MediaTrackConstraints page? These typedefs do include dictionaries with exact (required) and ideal (recommended) parameters for the constraint, and I feel it would be wrong not to document them in some way.
  • Where should we redirect the deleted pages to? (If the above question's answer is "yes", I think that pretty much answers this one.)

@hamishwillee
Copy link
Collaborator

@vinyldarkscratch Probably :-) Hard to say without trying it.

But yes, I think that is probably your best bet for minimising the amount of duplication and work. So I'd try to pull the definition of exact and ideal into that top level under deviceId and reference up to it from the mention in groupId. Same pattern for the others.

image

But of course it isn't that simple. What should you do in the property pages like https://developer.mozilla.org/en-US/docs/Web/API/MediaTrackConstraints/deviceId ? My thinking is that you'd TRY to link up to the the MediaTrackConstraints doc. However for usability you might decide that you'd be best off duplicating the format for the dictionary here too, and also in groupID. [That is what I think I'd end up doing]

Yes, you redirect all of these to MediaTrackConstraints.

Probably there will be some discussion once you have a PR. Sorry I can't be "canonical".

@queengooborg
Copy link
Collaborator

Thank you very much, this is a big help! I'm going to give a shot a PR for one of these typedefs and should have it up within the hour!

@queengooborg
Copy link
Collaborator

Ooh, actually, I just noticed that at the top of the page, there's a blurb that might just solve the issue altogether:

To learn more about how constraints work, see Capabilities, constraints, and settings.

The linked page seems to describe the constraint typedefs pretty well, documenting the exact and ideal properties. Perhaps we simply link and redirect to that page, considering the detail within the page?

@hamishwillee
Copy link
Collaborator

Generally I see guide and reference as separate - so even though this is a good explanation I wouldn't use it as the place where I send people to find out what the dictionaries are. Others might feel differently.

@sideshowbarker sideshowbarker added the Content:WebAPI Web API docs label Jun 15, 2021
@foolip
Copy link
Contributor

foolip commented Jun 16, 2021

Definitely agree this is a good thing to clean up!

Constraints have a very regular structure allowing both a "naked" value and as well as exact and ideal, so linking to an explanation of that structure in any documentation for methods taking constraint dictionaries seems important. https://developer.mozilla.org/en-US/docs/Web/API/Media_Streams_API/Constraints is a great page, but it doesn't directly and succinctly explain what the possible shapes of these constraint objects are, although it comes through in examples.

Is there any precedent we can follow here? I was looking for how we document callback functions as arguments, but setTimeout just links to Function, so no inspiration to find there.

@queengooborg
Copy link
Collaborator

I've opened #6047 as a proof-of-concept to get an idea for the removal. I'd love to get some feedback on it!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Content:WebAPI Web API docs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants