mirror of
https://github.com/streamyfin/streamyfin.git
synced 2026-07-12 15:32:54 +01:00
fix(chapters): address review findings + trickplay polish
Copilot + CodeRabbit review findings: - React.memo ChapterTicks and ChapterList (project guideline: hot-path components must use React.memo to cut redraw work during control updates). - chapterNameAt now sorts the chapter array once instead of twice per call. The previous version went through currentChapterIndex (chapterStartsMs + sort) then sortedChapters (sort again). Runs on every playback tick, so the duplicate work added up. - Import getUserLibraryApi from the public barrel (@jellyfin/sdk/lib/utils/api) instead of the deep internal path (@jellyfin/sdk/lib/utils/api/user-library-api) to match the rest of the codebase and avoid coupling to SDK file layout. TrickplayBubble polish: - Sit just above the slider (bottom: 0) so the bubble no longer overlaps the progress bar. - Move the chapter-name + timestamp overlay to the bottom-left of the preview frame, smaller font, in front of the surrounding overlays (zIndex + elevation). BottomControls cleanup: - Drop dev-only "pick one to test" comment in favour of a one-line note on TICK_HEIGHT. - Inline scrubMs into its useMemo callback so the scrub-chapter-name lookup only recomputes while a slide is active.
This commit is contained in:
@@ -9,7 +9,7 @@ import type {
|
|||||||
BaseItemDto,
|
BaseItemDto,
|
||||||
MediaSourceInfo,
|
MediaSourceInfo,
|
||||||
} from "@jellyfin/sdk/lib/generated-client/models";
|
} from "@jellyfin/sdk/lib/generated-client/models";
|
||||||
import { getUserLibraryApi } from "@jellyfin/sdk/lib/utils/api/user-library-api";
|
import { getUserLibraryApi } from "@jellyfin/sdk/lib/utils/api";
|
||||||
import { type Href } from "expo-router";
|
import { type Href } from "expo-router";
|
||||||
import { t } from "i18next";
|
import { t } from "i18next";
|
||||||
import { useAtom } from "jotai";
|
import { useAtom } from "jotai";
|
||||||
|
|||||||
@@ -6,7 +6,7 @@
|
|||||||
|
|
||||||
import { Ionicons } from "@expo/vector-icons";
|
import { Ionicons } from "@expo/vector-icons";
|
||||||
import type { ChapterInfo } from "@jellyfin/sdk/lib/generated-client/models";
|
import type { ChapterInfo } from "@jellyfin/sdk/lib/generated-client/models";
|
||||||
import { useEffect, useMemo, useRef } from "react";
|
import { memo, useEffect, useMemo, useRef } from "react";
|
||||||
import { useTranslation } from "react-i18next";
|
import { useTranslation } from "react-i18next";
|
||||||
import { FlatList, Modal, Pressable, StyleSheet, View } from "react-native";
|
import { FlatList, Modal, Pressable, StyleSheet, View } from "react-native";
|
||||||
import { Text } from "@/components/common/Text";
|
import { Text } from "@/components/common/Text";
|
||||||
@@ -30,7 +30,7 @@ interface ChapterListProps {
|
|||||||
|
|
||||||
const ROW_HEIGHT = 48;
|
const ROW_HEIGHT = 48;
|
||||||
|
|
||||||
export function ChapterList({
|
function ChapterListComponent({
|
||||||
visible,
|
visible,
|
||||||
chapters,
|
chapters,
|
||||||
currentPositionMs,
|
currentPositionMs,
|
||||||
@@ -151,6 +151,8 @@ export function ChapterList({
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export const ChapterList = memo(ChapterListComponent);
|
||||||
|
|
||||||
const styles = StyleSheet.create({
|
const styles = StyleSheet.create({
|
||||||
backdrop: {
|
backdrop: {
|
||||||
flex: 1,
|
flex: 1,
|
||||||
|
|||||||
@@ -4,7 +4,7 @@
|
|||||||
* so the slider underneath still receives touches.
|
* so the slider underneath still receives touches.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import { useState } from "react";
|
import { memo, useState } from "react";
|
||||||
import { type LayoutChangeEvent, PixelRatio, View } from "react-native";
|
import { type LayoutChangeEvent, PixelRatio, View } from "react-native";
|
||||||
import type { ChapterMarker } from "@/utils/chapters";
|
import type { ChapterMarker } from "@/utils/chapters";
|
||||||
|
|
||||||
@@ -19,7 +19,7 @@ interface ChapterTicksProps {
|
|||||||
width?: number;
|
width?: number;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function ChapterTicks({
|
function ChapterTicksComponent({
|
||||||
markers,
|
markers,
|
||||||
// Semi-transparent black contrasts against both the filled progress
|
// Semi-transparent black contrasts against both the filled progress
|
||||||
// (#fff) and the unfilled track (rgba(255,255,255,0.2)) so the ticks
|
// (#fff) and the unfilled track (rgba(255,255,255,0.2)) so the ticks
|
||||||
@@ -83,3 +83,5 @@ export function ChapterTicks({
|
|||||||
</View>
|
</View>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export const ChapterTicks = memo(ChapterTicksComponent);
|
||||||
|
|||||||
@@ -189,7 +189,6 @@ export const BottomControls: FC<BottomControlsProps> = ({
|
|||||||
onPress={() => setChapterListVisible(true)}
|
onPress={() => setChapterListVisible(true)}
|
||||||
hitSlop={10}
|
hitSlop={10}
|
||||||
className='justify-center mr-4'
|
className='justify-center mr-4'
|
||||||
style={{ marginTop: 10 }}
|
|
||||||
accessibilityRole='button'
|
accessibilityRole='button'
|
||||||
accessibilityLabel={t("chapters.open")}
|
accessibilityLabel={t("chapters.open")}
|
||||||
>
|
>
|
||||||
|
|||||||
@@ -52,8 +52,9 @@ export const TrickplayBubble: FC<TrickplayBubbleProps> = ({
|
|||||||
style={{
|
style={{
|
||||||
position: "absolute",
|
position: "absolute",
|
||||||
left: -62,
|
left: -62,
|
||||||
// Drop the bubble closer to the slider — less floating-high feel.
|
// Sit just above the slider — high enough not to overlap the
|
||||||
bottom: -20,
|
// progress bar, low enough to feel anchored to the thumb.
|
||||||
|
bottom: 0,
|
||||||
paddingTop: 12,
|
paddingTop: 12,
|
||||||
paddingBottom: 5,
|
paddingBottom: 5,
|
||||||
width: tileWidth * 1.5,
|
width: tileWidth * 1.5,
|
||||||
|
|||||||
@@ -70,9 +70,16 @@ export const chapterNameAt = (
|
|||||||
positionMs: number,
|
positionMs: number,
|
||||||
chapters: ChapterInfo[] | null | undefined,
|
chapters: ChapterInfo[] | null | undefined,
|
||||||
): string | null => {
|
): string | null => {
|
||||||
const idx = currentChapterIndex(positionMs, chapters);
|
// Sort once, derive both the active index and the entry from the same array
|
||||||
if (idx < 0) return null;
|
// — `chapterNameAt` runs on every playback tick, so paying for one `sort()`
|
||||||
|
// instead of two is worth the duplication of the index loop here.
|
||||||
const sorted = sortedChapters(chapters);
|
const sorted = sortedChapters(chapters);
|
||||||
|
let idx = -1;
|
||||||
|
for (let i = 0; i < sorted.length; i++) {
|
||||||
|
if (positionMs >= sorted[i].positionMs) idx = i;
|
||||||
|
else break;
|
||||||
|
}
|
||||||
|
if (idx < 0) return null;
|
||||||
const name = sorted[idx]?.chapter.Name;
|
const name = sorted[idx]?.chapter.Name;
|
||||||
return name && name.length > 0 ? name : null;
|
return name && name.length > 0 ? name : null;
|
||||||
};
|
};
|
||||||
|
|||||||
Reference in New Issue
Block a user