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

feat: Editorial review: Fenced Frames docs #30874

Merged
merged 36 commits into from
Mar 5, 2024

Conversation

chrisdavidmills
Copy link
Contributor

Description

#27781 contains the engineering technical review for my work on Fenced Frames docs, which has been completed and approved. Thank you to @gtanzer and others for your thorough and detailed review work.

This is a new PR based on the same branch, which is intended to contain the editorial review for the same work.


Background information

Fenced frames are an integral part of Google's privacy sandbox technologies. Many parts of this set are being made available by default in Chrome 115 (depending on a gradual ramp up to 100% of userbase over the 115 release period).

See my research document for more details of exactly what changes are expected in the PR.

Motivation

Additional details

Related issues and pull requests

@chrisdavidmills chrisdavidmills requested a review from a team as a code owner December 8, 2023 15:37
Copy link
Member

Choose a reason for hiding this comment

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

Any chance there are SVGs of these files? It's easier for maintenance & keeping bins out of the repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't provided with any, sorry. These came from the Chrome dev rel team. I'll ask around.

Copy link
Member

Choose a reason for hiding this comment

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

Would be awesome, not a blocker 🙏🏻

@bsmth
Copy link
Member

bsmth commented Mar 1, 2024

@chrisdavidmills I'm done taking a look for now, a couple of comments for your consideration 🙌🏻

@chrisdavidmills
Copy link
Contributor Author

@chrisdavidmills I'm done taking a look for now, a couple of comments for your consideration 🙌🏻

Thanks so much for the helpful comments, @bsmth! I have responded to all your comments so far.

Copy link
Member

@bsmth bsmth left a comment

Choose a reason for hiding this comment

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

Nice, thanks for incorporating the changes. Last thing is the images is there's SVG available, that would be great. Otherwise feel free to merge when you're ready!

Thank you!

See also:

image

@chrisdavidmills
Copy link
Contributor Author

Nice, thanks for incorporating the changes. Last thing is the images is there's SVG available, that would be great. Otherwise feel free to merge when you're ready!

Yay, thanks so much @bsmth! I am still chasing SVG images, but I won't block on that.

Thank you!

See also:

image

Well, y'know, this PR came from the dark days before I promised to limited PR sizes a bit ;-)

@chrisdavidmills chrisdavidmills merged commit 8a8e2a0 into mdn:main Mar 5, 2024
10 checks passed
@chrisdavidmills chrisdavidmills deleted the fenced-frames branch March 5, 2024 11:39
@bsmth
Copy link
Member

bsmth commented Mar 5, 2024

Well, y'know, this PR came from the dark days before I promised to limited PR sizes a bit ;-)

I presume you meant to hit 1337 additions, no more, no less 😄

@chrisdavidmills
Copy link
Contributor Author

Well, y'know, this PR came from the dark days before I promised to limited PR sizes a bit ;-)

I presume you meant to hit 1337 additions, no more, no less 😄

I did not even notice that; an even cooler PR to land than it was before ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:HTML Hypertext Markup Language docs Content:HTTP HTTP docs Content:WebAPI Web API docs size/xl [PR only] >1000 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants