Skip to content

Extend browsingContext.setViewport with devicePixelRatio#557

Merged
OrKoN merged 9 commits into
mainfrom
orkon/dpr
Oct 9, 2023
Merged

Extend browsingContext.setViewport with devicePixelRatio#557
OrKoN merged 9 commits into
mainfrom
orkon/dpr

Conversation

@OrKoN
Copy link
Copy Markdown
Contributor

@OrKoN OrKoN commented Sep 26, 2023 •
edited by pr-preview Bot
Loading

Closes #529

Preview | Diff
jgraham
jgraham reviewed Sep 26, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@jgraham jgraham left a comment

Choose a reason for hiding this comment

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

Added some comments. Formatting wise I think we usually have a blank line between list items.
Comment thread index.bs Outdated
Comment thread index.bs Outdated
Comment thread index.bs Outdated
@whimboo
Copy link
Copy Markdown
Contributor

whimboo commented Sep 26, 2023

Please also see the comment from the last TPAC meeting (2 weeks ago).
@OrKoN OrKoN changed the title WIP dpr [WIP] Extend browsingContext.setViewport with devicePixelRatio Sep 28, 2023
@OrKoN
Copy link
Copy Markdown
Contributor Author

OrKoN commented Sep 28, 2023 •
edited
Loading

@gsnedders I could not recall the last bits of the discussion that happened at TPAC and I wondered if you could expand on your comment about naming of the devicePixelRatio parameter?
@jrandolf-google
Copy link
Copy Markdown
Contributor

jrandolf-google commented Sep 29, 2023

In some fuzzy sense, browsingContext.setViewport is related to visualViewport, so I think it would make sense putting devicePixelRatio in a new method since it doesn't exist on visualViewport. In particular, devicePixelRatio is a property on window, so browsingContext.setDevicePixelRatio makes more sense.
@OrKoN OrKoN changed the title [WIP] Extend browsingContext.setViewport with devicePixelRatio Extend browsingContext.setViewport with devicePixelRatio Oct 5, 2023
@OrKoN OrKoN marked this pull request as ready for review October 6, 2023 09:30
@OrKoN OrKoN requested a review from whimboo October 6, 2023 09:30
@OrKoN
Copy link
Copy Markdown
Contributor Author

OrKoN commented Oct 6, 2023

Any thoughts on whether we want a separate method for configuring DPR? cc @whimboo @jgraham (see #557 (comment))
jgraham
jgraham requested changes Oct 6, 2023
View reviewed changes
Comment thread index.bs Outdated
Comment thread index.bs Outdated
@whimboo
Copy link
Copy Markdown
Contributor

whimboo commented Oct 6, 2023

Any thoughts on whether we want a separate method for configuring DPR? cc @whimboo @jgraham (see #557 (comment))

I'm fine with having it as part of the viewport emulation command as well.
@whimboo
Copy link
Copy Markdown
Contributor

whimboo commented Oct 6, 2023

@OrKoN I assume you are going to add some wpt tests for that new feature?
@OrKoN
Copy link
Copy Markdown
Contributor Author

OrKoN commented Oct 6, 2023

@whimboo yep, once there are no open discussions on the API structure for devicePixelRatio. Using the setViewport method sounds good to me too, to avoid roundtrip and multiple updates to the page layout.
@OrKoN
Copy link
Copy Markdown
Contributor Author

OrKoN commented Oct 6, 2023

Started WPT work here https://github.com/web-platform-tests/wpt/pull/42398/files
@OrKoN OrKoN requested a review from thiagowfx October 6, 2023 13:49
@jrandolf-google jrandolf-google self-requested a review October 6, 2023 15:08
thiagowfx
thiagowfx reviewed Oct 6, 2023
View reviewed changes
Comment thread index.bs Outdated
@OrKoN
Update index.bs
5563672 
Co-authored-by: Thiago Perrotta <tperrotta@chromium.org>
@OrKoN OrKoN merged commit af41c9b into main Oct 9, 2023
@OrKoN OrKoN deleted the orkon/dpr branch October 9, 2023 11:23
github-actions Bot added a commit that referenced this pull request Oct 9, 2023
@OrKoN @github-actions
Extend browsingContext.setViewport with devicePixelRatio (#557)
8c0dd40 
SHA: af41c9b
Reason: push, by OrKoN

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@OrKoN OrKoN mentioned this pull request Oct 11, 2023
Open
sadym-chromium pushed a commit that referenced this pull request Feb 25, 2025
@OrKoN @sadym-chromium
Extend browsingContext.setViewport with devicePixelRatio (#557)
ec98675 
Co-authored-by: Thiago Perrotta <tperrotta@chromium.org>
@gsnedders gsnedders mentioned this pull request Mar 18, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

5 participants

@OrKoN @whimboo @jrandolf-google @jgraham @thiagowfx