From d7ab553120c56368b348e3311a2ed2a31c1e4916 Mon Sep 17 00:00:00 2001 From: Rishi Ghan Date: Thu, 2 Apr 2026 13:07:13 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A8=20Clearning=20up=20the=20VolumeInf?= =?UTF-8?q?ormation=20tab?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .claude/skills/typescript/SKILL.md | 353 ++++++++++++++++++ .../components/ComicDetail/ComicDetail.tsx | 14 +- .../ComicDetail/Tabs/VolumeInformation.tsx | 107 +++++- .../components/ComicDetail/tabConfig.tsx | 28 +- 4 files changed, 472 insertions(+), 30 deletions(-) create mode 100644 .claude/skills/typescript/SKILL.md diff --git a/.claude/skills/typescript/SKILL.md b/.claude/skills/typescript/SKILL.md new file mode 100644 index 0000000..1379556 --- /dev/null +++ b/.claude/skills/typescript/SKILL.md @@ -0,0 +1,353 @@ +--- +name: typescript +description: TypeScript engineering guidelines based on Google's style guide. Use when writing, reviewing, or refactoring TypeScript code in this project. +--- + +Comprehensive guidelines for writing production-quality TypeScript based on Google's TypeScript Style Guide. +Naming Conventions +Type Convention Example +Classes, Interfaces, Types, Enums UpperCamelCase UserService, HttpClient +Variables, Parameters, Functions lowerCamelCase userName, processData +Global Constants, Enum Values CONSTANT_CASE MAX_RETRIES, Status.ACTIVE +Type Parameters Single letter or UpperCamelCase T, ResponseType +Naming Principles + + Descriptive names, avoid ambiguous abbreviations + Treat acronyms as words: loadHttpUrl not loadHTTPURL + No prefixes like opt_ for optional parameters + No trailing underscores for private properties + Single-letter variables only when scope is <10 lines + +Variable Declarations + +// Always use const by default +const users = getUsers(); + +// Use let only when reassignment is needed +let count = 0; +count++; + +// Never use var +// var x = 1; // WRONG + +// One variable per declaration +const a = 1; +const b = 2; +// const a = 1, b = 2; // WRONG + +Types and Interfaces +Prefer Interfaces Over Type Aliases + +// Good: interface for object shapes +interface User { + id: string; + name: string; + email?: string; +} + +// Avoid: type alias for object shapes +type User = { + id: string; + name: string; +}; + +// Type aliases OK for unions, intersections, mapped types +type Status = 'active' | 'inactive'; +type Combined = TypeA & TypeB; + +Type Inference + +Leverage inference for trivially inferred types: + +// Good: inference is clear +const name = 'Alice'; +const items = [1, 2, 3]; + +// Good: explicit for complex expressions +const result: ProcessedData = complexTransformation(input); + +Array Types + +// Simple types: use T[] +const numbers: number[]; +const names: readonly string[]; + +// Multi-dimensional: use T[][] +const matrix: number[][]; + +// Complex types: use Array +const handlers: Array<(event: Event) => void>; + +Null and Undefined + +// Prefer optional fields over union with undefined +interface Config { + timeout?: number; // Good + // timeout: number | undefined; // Avoid +} + +// Type aliases must NOT include |null or |undefined +type UserId = string; // Good +// type UserId = string | null; // WRONG + +// May use == for null comparison (catches both null and undefined) +if (value == null) { + // handles both null and undefined +} + +Types to Avoid + +// Avoid any - use unknown instead +function parse(input: unknown): Data { } + +// Avoid {} - use unknown, Record, or object +function process(obj: Record): void { } + +// Use lowercase primitives +let name: string; // Good +// let name: String; // WRONG + +// Never use wrapper objects +// new String('hello') // WRONG + +Classes +Structure + +class UserService { + // Fields first, initialized where declared + private readonly cache = new Map(); + private lastAccess: Date | null = null; + + // Constructor with parameter properties + constructor( + private readonly api: ApiClient, + private readonly logger: Logger, + ) {} + + // Methods separated by blank lines + async getUser(id: string): Promise { + // ... + } + + private validateId(id: string): boolean { + // ... + } +} + +Visibility + +class Example { + // private by default, only use public when needed externally + private internalState = 0; + + // readonly for properties never reassigned after construction + readonly id: string; + + // Never use #private syntax - use TypeScript visibility + // #field = 1; // WRONG + private field = 1; // Good +} + +Avoid Arrow Functions as Properties + +class Handler { + // Avoid: arrow function as property + // handleClick = () => { ... }; + + // Good: instance method + handleClick(): void { + // ... + } +} + +// Bind at call site if needed +element.addEventListener('click', () => handler.handleClick()); + +Static Methods + + Never use this in static methods + Call on defining class, not subclasses + +Functions +Prefer Function Declarations + +// Good: function declaration for named functions +function processData(input: Data): Result { + return transform(input); +} + +// Arrow functions when type annotation needed +const handler: EventHandler = (event) => { + // ... +}; + +Arrow Function Bodies + +// Concise body only when return value is used +const double = (x: number) => x * 2; + +// Block body when return should be void +const log = (msg: string) => { + console.log(msg); +}; + +Parameters + +// Use rest parameters, not arguments +function sum(...numbers: number[]): number { + return numbers.reduce((a, b) => a + b, 0); +} + +// Destructuring for multiple optional params +interface Options { + timeout?: number; + retries?: number; +} +function fetch(url: string, { timeout = 5000, retries = 3 }: Options = {}) { + // ... +} + +// Never name a parameter 'arguments' + +Imports and Exports +Always Use Named Exports + +// Good: named exports +export function processData() { } +export class UserService { } +export interface Config { } + +// Never use default exports +// export default class UserService { } // WRONG + +Import Styles + +// Module import for large APIs +import * as fs from 'fs'; + +// Named imports for frequently used symbols +import { readFile, writeFile } from 'fs/promises'; + +// Type-only imports when only used as types +import type { User, Config } from './types'; + +Module Organization + + Use modules, never namespace Foo { } + Never use require() - use ES6 imports + Use relative imports within same project + Avoid excessive ../../../ + +Control Structures +Always Use Braces + +// Good +if (condition) { + doSomething(); +} + +// Exception: single-line if +if (condition) return early; + +Loops + +// Prefer for...of for arrays +for (const item of items) { + process(item); +} + +// Use Object methods with for...of for objects +for (const [key, value] of Object.entries(obj)) { + // ... +} + +// Never use unfiltered for...in on arrays + +Equality + +// Always use === and !== +if (a === b) { } + +// Exception: == null catches both null and undefined +if (value == null) { } + +Switch Statements + +switch (status) { + case Status.Active: + handleActive(); + break; + case Status.Inactive: + handleInactive(); + break; + default: + // Always include default, even if empty + break; +} + +Exception Handling + +// Always throw Error instances +throw new Error('Something went wrong'); +// throw 'error'; // WRONG + +// Catch with unknown type +try { + riskyOperation(); +} catch (e: unknown) { + if (e instanceof Error) { + logger.error(e.message); + } + throw e; +} + +// Empty catch needs justification comment +try { + optional(); +} catch { + // Intentionally ignored: fallback behavior handles this +} + +Type Assertions + +// Use 'as' syntax, not angle brackets +const input = value as string; +// const input = value; // WRONG in TSX, avoid everywhere + +// Double assertion through unknown when needed +const config = (rawData as unknown) as Config; + +// Add comment explaining why assertion is safe +const element = document.getElementById('app') as HTMLElement; +// Safe: element exists in index.html + +Strings + +// Use single quotes for string literals +const name = 'Alice'; + +// Template literals for interpolation or multiline +const message = `Hello, ${name}!`; +const query = ` + SELECT * + FROM users + WHERE id = ? +`; + +// Never use backslash line continuations + +Disallowed Features +Feature Alternative +var const or let +Array() constructor [] literal +Object() constructor {} literal +any type unknown +namespace modules +require() import +Default exports Named exports +#private fields private modifier +eval() Never use +const enum Regular enum +debugger Remove before commit +with Never use +Prototype modification Never modify diff --git a/src/client/components/ComicDetail/ComicDetail.tsx b/src/client/components/ComicDetail/ComicDetail.tsx index 9026f8b..b27dfe9 100644 --- a/src/client/components/ComicDetail/ComicDetail.tsx +++ b/src/client/components/ComicDetail/ComicDetail.tsx @@ -130,6 +130,11 @@ export const ComicDetail = (data: ComicDetailProps): ReactElement => { const isComicBookMetadataAvailable = !isUndefined(comicvine) && !isUndefined(comicvine?.volumeInformation); + const hasAnyMetadata = + isComicBookMetadataAvailable || + !isEmpty(comicInfo) || + !isNil(locg); + const areRawFileDetailsAvailable = !isUndefined(rawFileDetails) && !isEmpty(rawFileDetails); @@ -147,16 +152,21 @@ export const ComicDetail = (data: ComicDetailProps): ReactElement => { }; // Create tab configuration + const openReconcilePanel = useCallback(() => { + setSlidingPanelContentId("metadataReconciliation"); + setVisible(true); + }, []); + const tabGroup = createTabConfig({ data: data.data, - comicInfo, - isComicBookMetadataAvailable, + hasAnyMetadata, areRawFileDetailsAvailable, airDCPPQuery, comicObjectId: _id, userSettings, issueName, acquisition, + onReconcileMetadata: openReconcilePanel, }); const filteredTabs = tabGroup.filter((tab) => tab.shouldShow); diff --git a/src/client/components/ComicDetail/Tabs/VolumeInformation.tsx b/src/client/components/ComicDetail/Tabs/VolumeInformation.tsx index 5bb28e0..b9ab459 100644 --- a/src/client/components/ComicDetail/Tabs/VolumeInformation.tsx +++ b/src/client/components/ComicDetail/Tabs/VolumeInformation.tsx @@ -1,15 +1,108 @@ -import React, { ReactElement } from "react"; +import React, { ReactElement, useMemo } from "react"; +import { isEmpty, isNil } from "lodash"; import ComicVineDetails from "../ComicVineDetails"; -export const VolumeInformation = (props): ReactElement => { - const { data } = props; +interface ComicVineMetadata { + volumeInformation?: Record; + name?: string; + number?: string; + resource_type?: string; + id?: number; +} + +interface SourcedMetadata { + comicvine?: ComicVineMetadata; + locg?: Record; + comicInfo?: unknown; + metron?: unknown; + gcd?: unknown; + [key: string]: unknown; +} + +interface VolumeInformationData { + sourcedMetadata?: SourcedMetadata; + inferredMetadata?: { issue?: unknown }; + updatedAt?: string; +} + +interface VolumeInformationProps { + data: VolumeInformationData; + onReconcile?: () => void; +} + +const SOURCED_METADATA_KEYS = ["comicvine", "locg", "comicInfo", "metron", "gcd"]; + +const SOURCE_LABELS: Record = { + comicvine: "ComicVine", + locg: "League of Comic Geeks", + comicInfo: "ComicInfo.xml", + metron: "Metron", + gcd: "Grand Comics Database", + inferredMetadata: "Local File", +}; + +const MetadataSourceChips = ({ + sources, + onReconcile, +}: { + sources: string[]; + onReconcile: () => void; +}): ReactElement => ( +
+ + + {sources.length} metadata sources detected + + {sources.map((source) => ( + + + {SOURCE_LABELS[source] ?? source} + + ))} + +
+); + +export const VolumeInformation = (props: VolumeInformationProps): ReactElement => { + const { data, onReconcile } = props; + + const presentSources = useMemo(() => { + const sources = SOURCED_METADATA_KEYS.filter((key) => { + const val = (data?.sourcedMetadata ?? {})[key]; + if (isNil(val) || isEmpty(val)) return false; + // locg returns an object even when empty; require at least one non-null value + if (key === "locg") return Object.values(val as Record).some((v) => !isNil(v) && v !== ""); + return true; + }); + if (!isNil(data?.inferredMetadata?.issue) && !isEmpty(data.inferredMetadata.issue)) { + sources.push("inferredMetadata"); + } + return sources; + }, [data?.sourcedMetadata, data?.inferredMetadata]); return (
- + {presentSources.length > 1 && ( + {})} + /> + )} + {presentSources.length === 1 && data.sourcedMetadata?.comicvine?.volumeInformation && ( + + )}
); }; diff --git a/src/client/components/ComicDetail/tabConfig.tsx b/src/client/components/ComicDetail/tabConfig.tsx index 8c006bc..06504f0 100644 --- a/src/client/components/ComicDetail/tabConfig.tsx +++ b/src/client/components/ComicDetail/tabConfig.tsx @@ -2,7 +2,6 @@ import React, { lazy } from "react"; import { isNil, isEmpty } from "lodash"; const VolumeInformation = lazy(() => import("./Tabs/VolumeInformation").then(m => ({ default: m.VolumeInformation }))); -const ComicInfoXML = lazy(() => import("./Tabs/ComicInfoXML").then(m => ({ default: m.ComicInfoXML }))); const ArchiveOperations = lazy(() => import("./Tabs/ArchiveOperations").then(m => ({ default: m.ArchiveOperations }))); const AcquisitionPanel = lazy(() => import("./AcquisitionPanel")); const TorrentSearchPanel = lazy(() => import("./TorrentSearchPanel")); @@ -18,26 +17,26 @@ interface TabConfig { interface TabConfigParams { data: any; - comicInfo: any; - isComicBookMetadataAvailable: boolean; + hasAnyMetadata: boolean; areRawFileDetailsAvailable: boolean; airDCPPQuery: any; comicObjectId: string; userSettings: any; issueName: string; acquisition?: any; + onReconcileMetadata?: () => void; } export const createTabConfig = ({ data, - comicInfo, - isComicBookMetadataAvailable, + hasAnyMetadata, areRawFileDetailsAvailable, airDCPPQuery, comicObjectId, userSettings, issueName, acquisition, + onReconcileMetadata, }: TabConfigParams): TabConfig[] => { return [ { @@ -46,23 +45,10 @@ export const createTabConfig = ({ icon: ( ), - content: isComicBookMetadataAvailable ? ( - + content: hasAnyMetadata ? ( + ) : null, - shouldShow: isComicBookMetadataAvailable, - }, - { - id: 2, - name: "ComicInfo.xml", - icon: ( - - ), - content: ( -
- {!isNil(comicInfo) && } -
- ), - shouldShow: !isEmpty(comicInfo), + shouldShow: hasAnyMetadata, }, { id: 3,