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

Add option to choose to take a screenshot with the device dimensions being the minimum. #1058

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

Conversation

Mounir-Bouaiche
Copy link

@Mounir-Bouaiche Mounir-Bouaiche commented Sep 3, 2023

✨ Pull Request

📓 Referenced Issue

ℹ️ About the PR

This is useful when you want to take a screenshot with the device dimensions being the minimum.

🖼️ Testing Scenarios / Screenshots

With:

iPhone 12 Pro-1693755429389

Without:

iPhone 12 Pro-1693755759659

@CLAassistant
Copy link

CLAassistant commented Sep 3, 2023

CLA assistant check
All committers have signed the CLA.

@Mounir-Bouaiche Mounir-Bouaiche changed the title Add option to choose not to modify the device's viewport dimensions Add option to choose not to modify the device's viewport dimensions when taking screen shot Sep 3, 2023
@Mounir-Bouaiche Mounir-Bouaiche changed the title Add option to choose not to modify the device's viewport dimensions when taking screen shot Add option to choose to take a screenshot with the device dimensions being the minimum. Sep 3, 2023
Copy link
Collaborator

@manojVivek manojVivek left a comment

Choose a reason for hiding this comment

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

This is a very thoughtful feature and glad you implemented it!
Just a couple of minor thoughts, looks great otherwise!

@@ -148,6 +148,10 @@ const schema = {
},
default: {},
},
onlyVisibleParts: {
type: 'boolean',
default: false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we make the default as true? So that the current behaviour stays the same unless specifically modified in the settings.


try {
if (rect != null) {
console.log(rect);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please remove this debug log line?

screenShotDelegate.setContentSize(rect.width, rect.height);
}
} catch (ex) {
//
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe console.log the error here?

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.

None yet

4 participants