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

@uppy/screen-capture: migrate to TS #4965

Merged
merged 5 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/@uppy/screen-capture/.npmignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@

Murderlon marked this conversation as resolved.
Show resolved Hide resolved
tsconfig.*
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
// TODO: remove this and type properly
/* eslint-disable @typescript-eslint/explicit-module-boundary-types */
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-nocheck
Copy link
Member

Choose a reason for hiding this comment

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

Given that we don't add any type, shouldn't we keep this file as .jsx?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought it would be nicer for the diff next time and easier for the next person to get started.

Copy link
Member

Choose a reason for hiding this comment

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

The following seems to work just fine as long as you type the props as any:

Suggested change
// TODO: remove this and type properly
/* eslint-disable @typescript-eslint/explicit-module-boundary-types */
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-nocheck
/* eslint-disable @typescript-eslint/explicit-module-boundary-types */

Maybe it's better so we at least get type safety for HTML/Preact attributes?

Copy link
Member Author

@Murderlon Murderlon Feb 27, 2024

Choose a reason for hiding this comment

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

I wouldn't for now as it still requires significant work for bigger UI plugins. We're down one person and everyone is part-time, I think we can justify this speed up, while not sacrificing user facing types.

import { h } from 'preact'

/**
* Control screen capture recording. Will show record or stop button.
*/
export default function RecordButton ({ recording, onStartRecording, onStopRecording, i18n }) {
export default function RecordButton({
recording,
onStartRecording,
onStopRecording,
i18n,
}) {
if (recording) {
return (
<button
Expand All @@ -14,7 +23,14 @@ export default function RecordButton ({ recording, onStartRecording, onStopRecor
onClick={onStopRecording}
data-uppy-super-focusable
>
<svg aria-hidden="true" focusable="false" className="uppy-c-icon" width="100" height="100" viewBox="0 0 100 100">
<svg
aria-hidden="true"
focusable="false"
className="uppy-c-icon"
width="100"
height="100"
viewBox="0 0 100 100"
>
<rect x="15" y="15" width="70" height="70" />
</svg>
</button>
Expand All @@ -30,7 +46,14 @@ export default function RecordButton ({ recording, onStartRecording, onStopRecor
onClick={onStartRecording}
data-uppy-super-focusable
>
<svg aria-hidden="true" focusable="false" className="uppy-c-icon" width="100" height="100" viewBox="0 0 100 100">
<svg
aria-hidden="true"
focusable="false"
className="uppy-c-icon"
width="100"
height="100"
viewBox="0 0 100 100"
>
<circle cx="50" cy="50" r="40" />
</svg>
</button>
Expand Down
58 changes: 0 additions & 58 deletions packages/@uppy/screen-capture/src/RecorderScreen.jsx

This file was deleted.

87 changes: 87 additions & 0 deletions packages/@uppy/screen-capture/src/RecorderScreen.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/* eslint-disable react/jsx-props-no-spreading */
import { h, Component, type ComponentChild } from 'preact'
import type { Body, Meta } from '@uppy/utils/lib/UppyFile'
import RecordButton from './RecordButton.tsx'
import SubmitButton from './SubmitButton.tsx'
import StopWatch from './StopWatch.tsx'
import StreamStatus from './StreamStatus.tsx'

import ScreenCapture, { type ScreenCaptureState } from './ScreenCapture.tsx'

type RecorderScreenProps<M extends Meta, B extends Body> = {
onStartRecording: ScreenCapture<M, B>['startRecording']
onStopRecording: ScreenCapture<M, B>['stopRecording']
onStop: ScreenCapture<M, B>['stop']
onSubmit: ScreenCapture<M, B>['submit']
i18n: ScreenCapture<M, B>['i18n']
stream: ScreenCapture<M, B>['videoStream']
} & ScreenCaptureState

class RecorderScreen<M extends Meta, B extends Body> extends Component<
RecorderScreenProps<M, B>
> {
videoElement: HTMLVideoElement | null

componentWillUnmount(): void {
const { onStop } = this.props
onStop()
}

render(): ComponentChild {
const { recording, stream: videoStream, recordedVideo } = this.props

const videoProps: {
muted?: boolean
autoplay?: boolean
playsinline?: boolean
controls?: boolean
src?: string
srcObject?: MediaStream | null
} = {
playsinline: true,
}

// show stream
if (recording || (!recordedVideo && !recording)) {
videoProps.muted = true
videoProps.autoplay = true
videoProps.srcObject = videoStream
}

// show preview
if (recordedVideo && !recording) {
videoProps.muted = false
videoProps.controls = true
videoProps.src = recordedVideo

// reset srcObject in dom. If not resetted, stream sticks in element
if (this.videoElement) {
this.videoElement.srcObject = null
}
}

return (
<div className="uppy uppy-ScreenCapture-container">
<div className="uppy-ScreenCapture-videoContainer">
<StreamStatus {...this.props} />
{/* eslint-disable-next-line jsx-a11y/media-has-caption */}
<video
ref={(videoElement) => {
this.videoElement = videoElement
}}
className="uppy-ScreenCapture-video"
{...videoProps}
/>
<StopWatch {...this.props} />
</div>

<div className="uppy-ScreenCapture-buttonContainer">
<RecordButton {...this.props} />
<SubmitButton {...this.props} />
</div>
</div>
)
}
}

export default RecorderScreen