Skip to content

Feature/arc 3904 video player - #150

Open
reunefe wants to merge 12 commits into
masterfrom
feature/ARC-3904_video-player
Open

Feature/arc 3904 video player#150
reunefe wants to merge 12 commits into
masterfrom
feature/ARC-3904_video-player

Conversation

@reunefe

@reunefe reunefe commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@reunefe
reunefe requested a review from bertyhell September 4, 2026 06:59
Comment thread src/components/AudioWaveFormDisplay/AudioWaveFormDisplay.helpers.ts
@@ -0,0 +1,75 @@
export type AudioWaveFormDisplaySize = 'small' | 'large';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i always prefer enums, since that links the usage and types together
'small' vs AudioWaveFormDisplaySize.Small
it's easier to search for occurances for the latter

Comment thread src/components/FlowPlayer/Controls/ControlBar.scss
Comment thread src/components/FlowPlayer/Controls/ControlBar.scss
Comment thread src/components/FlowPlayer/Controls/ControlBar.scss
Comment thread src/components/FlowPlayer/Controls/ControlBar.scss
Comment thread src/components/FlowPlayer/Controls/ControlBar.scss

import './ControlBar.scss';

type FlyoutId = 'subtitles' | 'speed';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

prefer enum

Comment on lines +23 to +35
export const defaultControlsLabels: Required<FlowPlayerControlsLabels> = {
play: 'Afspelen',
pause: 'Pauzeren',
mute: 'Dempen',
unmute: 'Dempen opheffen',
volume: 'Volume',
fullscreenEnter: 'Volledig scherm',
fullscreenExit: 'Volledig scherm sluiten',
subtitles: 'Ondertitels',
subtitlesOff: 'Uit',
speed: 'Snelheid',
progressBar: 'Voortgang',
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we want the client to manage the translations or the component library?
for the rich text we allow the locale to be set
and the labels are hardcoded in the component:

We should use the same mechanism, or also switch the rich text to this mechanism
i kind of like the simplicity of only passing the locale

// meaning, just applied to this visual instead of the canvas; `active`/`inactive` are revealed
// via the same clip-path progress technique already used for the progress bar's own light/dark
// label overlay.
peakMode?: 'data' | 'generic'; // default 'data'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we need this?
can't we just have a property: peakData?: any[];

and show the generic when it's undefined?

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.

2 participants