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

Limit emulation to enabling slowmoable #331

Merged
merged 1 commit into from
Jul 14, 2023

Conversation

aaronjensen
Copy link
Contributor

Fixes #330

If the line cannot be removed (because slowmoable needs to stay enabled) then this will disable the override of everything else.

@aaronjensen
Copy link
Contributor Author

@route would you be able to take a look at this please?

@route
Copy link
Member

route commented Jul 13, 2023

@aaronjensen could you please rebase it with master? I don't see how to run tests for this PR

@aaronjensen
Copy link
Contributor Author

Rebased. When you say you don't see how to run tests, are you saying that you don't see any tests or you this PR somehow breaks the normal means of running tests? The emulation device metric overrides were not previously tested afaict, so I wasn't sure how to add a test for them.

@aaronjensen
Copy link
Contributor Author

I do see a test failure actually in browser_spec.rb, specifically:

include_examples "resize viewport by fullscreen" do
  let(:path) { "/ferrum/custom_html_size_100%" }
  let(:viewport_size) { [1272, 1008] }
end

@route
Copy link
Member

route commented Jul 13, 2023

@aaronjensen I was talking about a button to "Approve and Run tests" for this PR. Now it appeared. We need to investigate this failure. Cannot merge it with a breaking test ;)

@aaronjensen
Copy link
Contributor Author

Updated to only disable overriding the device scale factor. Setting the height and width still work and are necessary to allow controlling height and width when using full screen.

@route
Copy link
Member

route commented Jul 14, 2023

Looks good now! Sorry that it took me so long! ❤️

@route route merged commit 783d087 into rubycdp:main Jul 14, 2023
10 checks passed
@aaronjensen aaronjensen deleted the limit-emulation branch July 14, 2023 13:39
@aaronjensen
Copy link
Contributor Author

Thank you!

@aaronjensen
Copy link
Contributor Author

@route sorry to bother you again, but do you have an idea when you plan to release a new gem version with this change? Thank you!

@route
Copy link
Member

route commented Aug 1, 2023

I don’t know, when it’s time :) use master

@aaronjensen
Copy link
Contributor Author

Understood, thanks for the response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Emulation.setDeviceMetricsOverride causes wrong scale to be used for screenshots
2 participants