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 scaleFactor to CLI | Add .cancel function #20

Closed

Conversation

dac09
Copy link

@dac09 dac09 commented May 14, 2021

What does it do?

  • Adds scale factor argument to CLI and node function
  • Adds new cancel function to stop and discard the recording

Fixes #18
Please note I haven't had a chance to test this out yet on this version, but I am using something similar in my older fork

index.d.ts Outdated Show resolved Hide resolved
index.d.ts Outdated
/**
Scale factor to use
Useful when recording retina screens, or for resizing the recorded video
*/
Copy link
Member

Choose a reason for hiding this comment

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

Incorrect indentation.

Copy link
Author

Choose a reason for hiding this comment

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

Is it worth adding prettier to the project? Would avoid these sort of issues.

Copy link
Member

Choose a reason for hiding this comment

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

Nah. prettier does indeed make things more consistent, but IMHO also less readable and ugly.

index.d.ts Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.d.ts Outdated
/**
Cancels a recording, if it was started
*/
cancelRecording: () => void;
Copy link
Member

Choose a reason for hiding this comment

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

I just noticed that we have been very inconsistent with naming. I think this should be cancel(). I'll fix the other methods when this is merged.

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Member

You need to update the readme too.

@sindresorhus
Copy link
Member

For next time, the .cancel() addition should have been a separate pull request.

@dac09 dac09 changed the title Add scaleFactor to CLI | Add .cancelRecording function Add scaleFactor to CLI | Add .cancel function May 14, 2021
@dac09 dac09 marked this pull request as ready for review June 10, 2021 17:51
Comment on lines +42 to +43

recorderTimeout = undefined
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
recorderTimeout = undefined
_recorderTimeout = undefined

@@ -101,6 +108,11 @@ declare namespace aperture {
Returns a `Promise` for the path to the screen recording file.
*/
stopRecording: () => Promise<string>;

/**
Cancels a recording, if it was started
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Cancels a recording, if it was started
Cancels a recording if it was running.

@sindresorhus
Copy link
Member

Readme and index.d.ts should be fully in sync.

Comment on lines +66 to +68
Scale factor to use
The actual height and width of the capture will be multiplied by this number to create the height and width of the output.
@default 1
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Scale factor to use
The actual height and width of the capture will be multiplied by this number to create the height and width of the output.
@default 1
The scale factor to use.
The actual height and width of the capture will be multiplied by this number to create the height and width of the output.
@default 1

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.

Add scaleFactor options
2 participants