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

new subtitle text colors #96

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 49 additions & 18 deletions src/components/player/atoms/settings/CaptionSettingsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Icon, Icons } from "@/components/Icon";
import { Menu } from "@/components/player/internals/ContextMenu";
import { useOverlayRouter } from "@/hooks/useOverlayRouter";
import { useProgressBar } from "@/hooks/useProgressBar";
import { useSubtitleStore } from "@/stores/subtitles";
import { SubtitleStyling, useSubtitleStore } from "@/stores/subtitles";

export function ColorOption(props: {
color: string;
Expand Down Expand Up @@ -214,7 +214,7 @@ export function CaptionSetting(props: {
);
}

export const colors = ["#ffffff", "#b0b0b0", "#80b1fa", "#e2e535"];
export const colors = ["#ffffff", "#80b1fa", "#e2e535", "#10B239FF"];
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a fan of these new colors


export function CaptionSettingsView({
id,
Expand All @@ -225,12 +225,21 @@ export function CaptionSettingsView({
}) {
const { t } = useTranslation();
const router = useOverlayRouter(id);
const styling = useSubtitleStore((s) => s.styling);
const overrideCasing = useSubtitleStore((s) => s.overrideCasing);
const delay = useSubtitleStore((s) => s.delay);
const setOverrideCasing = useSubtitleStore((s) => s.setOverrideCasing);
const setDelay = useSubtitleStore((s) => s.setDelay);
const updateStyling = useSubtitleStore((s) => s.updateStyling);
const subtitleStore = useSubtitleStore();
Copy link
Member

Choose a reason for hiding this comment

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

I think there was a reason we didn't do this earlier and called the function every time

const styling = subtitleStore.styling;
const overrideCasing = subtitleStore.overrideCasing;
const delay = subtitleStore.delay;
const setOverrideCasing = subtitleStore.setOverrideCasing;
const setDelay = subtitleStore.setDelay;
const updateStyling = subtitleStore.updateStyling;

useEffect(() => {
subtitleStore.updateStyling(styling);
}, [styling, subtitleStore]);

const handleStylingChange = (newStyling: SubtitleStyling) => {
updateStyling(newStyling);
};

return (
<>
Expand Down Expand Up @@ -270,7 +279,9 @@ export function CaptionSettingsView({
<div className="flex justify-center items-center">
<Toggle
enabled={styling.bold}
onClick={() => updateStyling({ bold: !styling.bold })}
onClick={() =>
handleStylingChange({ ...styling, bold: !styling.bold })
}
/>
</div>
</div>
Expand All @@ -279,15 +290,19 @@ export function CaptionSettingsView({
label={t("settings.subtitles.backgroundLabel")}
max={100}
min={0}
onChange={(v) => updateStyling({ backgroundOpacity: v / 100 })}
onChange={(v) =>
handleStylingChange({ ...styling, backgroundOpacity: v / 100 })
}
value={styling.backgroundOpacity * 100}
textTransformer={(s) => `${s}%`}
/>
<CaptionSetting
label={t("settings.subtitles.backgroundBlurLabel")}
max={100}
min={0}
onChange={(v) => updateStyling({ backgroundBlur: v / 100 })}
onChange={(v) =>
handleStylingChange({ ...styling, backgroundBlur: v / 100 })
}
value={styling.backgroundBlur * 100}
textTransformer={(s) => `${s}%`}
/>
Expand All @@ -296,22 +311,38 @@ export function CaptionSettingsView({
max={200}
min={1}
textTransformer={(s) => `${s}%`}
onChange={(v) => updateStyling({ size: v / 100 })}
onChange={(v) => handleStylingChange({ ...styling, size: v / 100 })}
value={styling.size * 100}
/>
<div className="flex justify-between items-center">
<Menu.FieldTitle>
{t("settings.subtitles.colorLabel")}
</Menu.FieldTitle>
<div className="flex justify-center items-center">
{colors.map((v) => (
<div className="flex justify-center items-center space-x-2">
{colors.map((color) => (
<ColorOption
onClick={() => updateStyling({ color: v })}
color={v}
active={styling.color === v}
key={v}
key={color}
color={color}
active={styling.color === color}
onClick={() => handleStylingChange({ ...styling, color })}
/>
))}
{/* Add Color Picker */}
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using the default browser color picker we should use a lib like react-colorful or something

<div className="relative inline-block">
<input
type="color"
value={styling.color}
onChange={(e) => {
const color = e.target.value;
handleStylingChange({ ...styling, color });
}}
className="absolute opacity-0 cursor-pointer w-10 h-10"
/>
<div
className="w-10 h-10 border-2 border-gray-300 rounded-full"
style={{ backgroundColor: styling.color }}
/>
</div>
</div>
</div>
</Menu.Section>
Expand Down
59 changes: 51 additions & 8 deletions src/pages/parts/settings/CaptionsPart.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import classNames from "classnames";
import { useState } from "react";
import { useEffect, useState } from "react";
import { Helmet } from "react-helmet-async";
import { useTranslation } from "react-i18next";

Expand All @@ -14,7 +14,7 @@ import { Menu } from "@/components/player/internals/ContextMenu";
import { CaptionCue } from "@/components/player/Player";
import { Heading1 } from "@/components/utils/Text";
import { Transition } from "@/components/utils/Transition";
import { SubtitleStyling } from "@/stores/subtitles";
import { SubtitleStyling, useSubtitleStore } from "@/stores/subtitles";

export function CaptionPreview(props: {
fullscreen?: boolean;
Expand Down Expand Up @@ -78,6 +78,17 @@ export function CaptionsPart(props: {
const { t } = useTranslation();
const [fullscreenPreview, setFullscreenPreview] = useState(false);

const subtitleStore = useSubtitleStore();

useEffect(() => {
subtitleStore.updateStyling(props.styling);
}, [props.styling, subtitleStore, subtitleStore.updateStyling]);

const handleStylingChange = (newStyling: SubtitleStyling) => {
props.setStyling(newStyling);
subtitleStore.updateStyling(newStyling);
};

return (
<div>
<Heading1 border>{t("settings.subtitles.title")}</Heading1>
Expand All @@ -88,7 +99,10 @@ export function CaptionsPart(props: {
max={100}
min={0}
onChange={(v) =>
props.setStyling({ ...props.styling, backgroundOpacity: v / 100 })
handleStylingChange({
...props.styling,
backgroundOpacity: v / 100,
})
}
value={props.styling.backgroundOpacity * 100}
textTransformer={(s) => `${s}%`}
Expand All @@ -98,7 +112,10 @@ export function CaptionsPart(props: {
max={100}
min={0}
onChange={(v) =>
props.setStyling({ ...props.styling, backgroundBlur: v / 100 })
handleStylingChange({
...props.styling,
backgroundBlur: v / 100,
})
}
value={props.styling.backgroundBlur * 1}
textTransformer={(s) => `${s}%`}
Expand All @@ -109,7 +126,10 @@ export function CaptionsPart(props: {
min={1}
textTransformer={(s) => `${s}%`}
onChange={(v) =>
props.setStyling({ ...props.styling, size: v / 100 })
handleStylingChange({
...props.styling,
size: v / 100,
})
}
value={props.styling.size * 100}
/>
Expand All @@ -121,7 +141,7 @@ export function CaptionsPart(props: {
<Toggle
enabled={props.styling.bold}
onClick={() =>
props.setStyling({
handleStylingChange({
...props.styling,
bold: !props.styling.bold,
})
Expand All @@ -133,17 +153,40 @@ export function CaptionsPart(props: {
<Menu.FieldTitle>
{t("settings.subtitles.colorLabel")}
</Menu.FieldTitle>
<div className="flex justify-center items-center">
<div className="flex justify-center items-center space-x-2">
{colors.map((v) => (
<ColorOption
onClick={() =>
props.setStyling({ ...props.styling, color: v })
handleStylingChange({
...props.styling,
color: v,
})
}
color={v}
active={props.styling.color === v}
key={v}
/>
))}
{/* Add Color Picker */}
Copy link
Member

Choose a reason for hiding this comment

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

It's better to make the color picker a component that can be used anywhere

<div className="relative">
<input
type="color"
value={props.styling.color}
onChange={(e) => {
const color = e.target.value;
handleStylingChange({ ...props.styling, color });
subtitleStore.updateStyling({
...props.styling,
color,
});
}}
className="absolute opacity-0 cursor-pointer w-8 h-8"
/>
<div
className="w-8 h-8 border-2 border-gray-300 rounded-full"
style={{ backgroundColor: props.styling.color }}
/>
</div>
</div>
</div>
</div>
Expand Down