Skip to content

Improve description of region capture API #2902

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

foolip
Copy link
Collaborator

@foolip foolip commented Apr 25, 2025

These are changes suggested by @eladalon1983 after I asked about the current description, which I thought seemed wrong in parts.

@github-actions github-actions bot added the feature definition Creating or defining new features or groups of features. label Apr 25, 2025
@foolip foolip requested a review from ddbeck April 25, 2025 09:58
Comment on lines +2 to +11
description: |
Region Capture is an API for cropping video tracks derived from screen-capturing a tab.

The API consists of two parts.

First, a `CropTarget` object is minted from an HTMLElement `e`, representing an element within the captured tab.

Second, `cropTo()` is invoked on the video track, with the aforementioned `CropTarget` as input, after which frames deliver on that track will be cropped to `e`'s bounding box.

Calling `cropTo(null)` restores the track to its original, uncropped form.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wrote the original description. It's not great, but I don't think it's wrong. CropTarget is the unavoidable entry point, as far as I can tell.

Here are two new variations for your consideration. The first tries to illustrate the relationship between CropTarget and the video track:

Suggested change
description: |
Region Capture is an API for cropping video tracks derived from screen-capturing a tab.
The API consists of two parts.
First, a `CropTarget` object is minted from an HTMLElement `e`, representing an element within the captured tab.
Second, `cropTo()` is invoked on the video track, with the aforementioned `CropTarget` as input, after which frames deliver on that track will be cropped to `e`'s bounding box.
Calling `cropTo(null)` restores the track to its original, uncropped form.
description: The `CropTarget.fromElement()` static method selects an element to be cropped from a display-capture video track by the track's `cropTo()` method. For example, use it to crop out video conferencing controls during screen sharing.

This one throws up its hands and says, it's complicated:

Suggested change
description: |
Region Capture is an API for cropping video tracks derived from screen-capturing a tab.
The API consists of two parts.
First, a `CropTarget` object is minted from an HTMLElement `e`, representing an element within the captured tab.
Second, `cropTo()` is invoked on the video track, with the aforementioned `CropTarget` as input, after which frames deliver on that track will be cropped to `e`'s bounding box.
Calling `cropTo(null)` restores the track to its original, uncropped form.
description: The region capture API crops an element, such as an element containing video conferencing controls, from a captured video track.

Choose a reason for hiding this comment

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

The proposal is shorter and has some benefits, but I worry that it could be misunderstood, especially around the word "select". Calling CropTarget.fromElement() itself has no effect; one could call it multiple times with different inputs and see no change. It's only when the returned token is fed into cropTo() that an effect follows. If the suggestion could be modified to better reflect this, I think that would be best.

Copy link

@eladalon1983 eladalon1983 Apr 30, 2025

Choose a reason for hiding this comment

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

"crops an element ... from"

With this suggestion, I worry that the reader could get the impression that the element is removed from the capture, whereas it's actually everything else that gets removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature definition Creating or defining new features or groups of features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants