proto: Paratext 10 Simple workspace component ideation (Saroj studies)#2275
Open
merchako wants to merge 39 commits into
Open
proto: Paratext 10 Simple workspace component ideation (Saroj studies)#2275merchako wants to merge 39 commits into
merchako wants to merge 39 commits into
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Creates the ResourcePickerDialog React component in platform-bible-react
with search filtering, language filtering, and three conditional sections
(already selected, installed, available to download). Adapts test traversal
to use closest('tr') for table row button lookup. All 11 tests pass.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add localization strings to en.json and es.json - Make allResources optional to fix DialogDefinition type compatibility - Remove table row borders and hover highlights - Fix non-null assertion ESLint violations in tests - Add JSDoc to exported component types - Fix z-index on MultiSelectComboBox in Storybook - Rebuild platform-bible-react dist Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Regenerate papi.d.ts (allResources now correctly optional) - Fix en.json key ordering (resourcePicker* before resources_deprecated_any) - Extract ResourceSection component to eliminate duplicated JSX - Fix NoResults story to use allResources: [] for true empty state - Fix sentence case: "Already selected", "Available to download", "Resource picker" - Remove hover:tw-bg-transparent from TableCell (restore standard hover highlight) - Fix ellipsis in search placeholder (... → …) across en.json, es.json, stories, test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Create resource-picker-dialog.utils.test.ts with 4 test cases - Tests check initial state, pagination, reset on item change, and hasMore flag - Tests use Vitest mocks for IntersectionObserver - All tests fail as expected (module not found) - RED phase of TDD Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…to Download section
…fecycle and add missing negative test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…language picker - Add missing Spanish translation for %resourcePicker_language_filter_multipleSelected% - Fix customLanguageSelectText useMemo missing anyLanguageText and localizedStrings deps - Remove redundant `as IntersectionObserverEntry` casts in utils test - Add JSDoc to useProgressiveList documenting sentinelRef placement and memoization contract - Add aria-hidden to scroll sentinel div - Add explicit name to LargeResourceList Storybook story - Regenerate dist build artifacts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Resolved conflict on workspace-shell.tsx by taking Agent B's version (slightly
more complete docstring, default export, key={label}).
Agents A and B used the orchestrator's documented `tw-` prefix (Tailwind v3 syntax). The codebase moved to Tailwind v4 in the b6rt8cvlC shadcn preset upgrade — the prefix is now `tw:` (variant-style colon). `tw-flex` etc. would silently produce no styles. Global replace across the proto/saroj-studies files; pure prefix swap, no logic changes. https://claude.ai/code/session_01FN5PduQzmUwiTD28mytLpN
merchako
requested review from
irahopkinson,
jolierabideau,
lyonsil,
rolfheij-sil and
tjcouch-sil
as code owners
Post-merge format pass. Agent A ran prettier in its worktree, but the post-merge tw- -> tw: prefix swap re-introduced minor wrapping issues (short attribute strings became long enough to require wrap, etc.). No content changes, just whitespace/wrapping. Agent B and Agent C files already pass prettier --check. https://claude.ai/code/session_01FN5PduQzmUwiTD28mytLpN
CI reported 10 ESLint errors on no-project-zero-state.component.tsx: - Variant function names used PascalCase + underscore (VariantA_X); renamed to VariantAX across component and stories (camelcase rule). - 'React' not defined — added 'import * as React from "react"'. - Three eslint-disable-next-line no-console comments missing the '--' explanation required by eslint-comments/require-description; added. - onCreateProject prop flagged as unused because Variant A intentionally omits the affordance (registry-only flow). Documented the per-variant intent in the JSDoc and added a targeted eslint-disable for react/no-unused-prop-types. No story behavior changes. https://claude.ai/code/session_01FN5PduQzmUwiTD28mytLpN
CI's second pass turned up two more eslint issues:
- Story const names A_RegistryRequired / B_LocalCreation / C_Gallery
(plus the …WithLocalProjects siblings) also tripped the camelcase rule.
Renamed to ARegistryRequired etc. The visible Storybook story names
come from the `name:` field which is unchanged ("A · Registry-required"
etc.), so reviewers see the same labels.
- The no-console eslint-disable directives in this file were flagged as
unnecessary (the rule doesn't fire in stories / on these handler stubs
by config). Removed all five. The console.log calls remain — they're
prototype handler stubs that log what the real wiring would do.
react/no-unused-prop-types disable on onCreateProject kept; that one IS
needed.
https://claude.ai/code/session_01FN5PduQzmUwiTD28mytLpN
CI's third pass reported 10 errors and several warnings: - `react/no-unused-prop-types` flagged shared interfaces (ZeroStateProps, PopulatedTabProps) when one variant didn't use a prop. Added per-prop eslint-disable comments with JSDoc explaining the per-variant intent. - `react/jsx-fragments` flagged `<></>` with one child in the inline picker `if (!open)` early return; returned `null` instead. - `no-restricted-syntax` (or similar) flagged a `for…of` loop in `model-text-zero-state.utils.ts`; switched to two `filter()` calls. - `react/no-array-index-key` flagged using the loop index as a key on the placeholder lines in `ModelTextZeroStateInline`; the percentages themselves are stable and unique, so keyed by value. - `'React' is not defined` in `model-text-picker.component.tsx`; added `import * as React from 'react'` alongside the existing named hook imports. - `import/no-named-as-default` flagged three story files that imported the default export of `workspace-shell` even though the file also has a matching named export. Switched to named imports. Console-log warnings on the default handler stubs in `no-project-zero-state.component.tsx` remain (lines 94/97/100). They are intentional in prototype handler stubs and are warnings, not errors, so they do not fail CI. https://claude.ai/code/session_01FN5PduQzmUwiTD28mytLpN
Last commit changed the inline picker's `if (!open)` early return from `<></>` to `null`. That broke the structural type compatibility with the other two picker variants — the stories pass picker components around typed as `typeof ScriptureResourcesPickerModalChecklist`, and the Modal and TwoPanel variants both return `Element` unconditionally. Returning `<div hidden />` gives us a real Element while still being visually a no-op, and doesn't trip `react/jsx-no-useless-fragment`. https://claude.ai/code/session_01FN5PduQzmUwiTD28mytLpN
Split variant is the inline picker itself; it has no full-picker handoff and intentionally doesn't read onOpenPicker. Standard and Suggestions still use it. Added eslint-disable with JSDoc explanation. Note: macOS "check packaging" failure is unrelated to lint (electron-builder step). Will diagnose separately if it recurs. https://claude.ai/code/session_01FN5PduQzmUwiTD28mytLpN
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UX ideation pass for Paratext 10 Simple workspace components. Three parallel sub-agents each built component variants in their own worktree; this PR is the merge of all three. Branched off
pt-3976-shared-resource-picker-uiso the existingResourcePickerDialogsample data and field shapes are in scope.This is prototype-fidelity work, intended for the June 19 UBS-consultants demo. Not for production. No backend changes.
What's in here
Agent A — Model Text column (Column 1) at
lib/platform-bible-react/src/components/advanced/model-text-zero-state/Advanced/ModelTextZeroState,Advanced/ModelTextPickerAgent B — Scripture Resources tab (Column 3, Bible Texts only) at
lib/platform-bible-react/src/components/advanced/scripture-resources-tab/Advanced/ScriptureResourcesZeroState,Advanced/ScriptureResourcesTab,Advanced/ScriptureResourcesPickerforceRemoveTooltipOpenForIdstory prop so tooltip review doesn't need hoverAgent C — Full-screen no-project zero state (app-level) at
src/stories/platform/no-project-zero-state/Platform/NoProjectZeroStatehasLocalProjectstoggle)@storybook/react-webpack5(consistent with rest of root app Storybook)Shared additions
lib/platform-bible-react/src/storybook/decorators/workspace-shell.tsx— 3-column shell with stub BCV toolbar and icon-tab stub in Column 3 (PRD: tabs must NOT mimic 10Power chrome / must NOT look draggable)tw:used throughout (the v2 prompt incorrectly documentedtw-; corrected in a follow-up commit)Data shape
Real
DblResourceDatafromplatform-bible-utils(dblEntryUid,displayName,bestLanguageName,installed,type, …). ReusesSAMPLE_RESOURCESfrom the in-flight picker onpt-3976-shared-resource-picker-ui. "Admin vs user" association is modeled as parallel string[] (adminAssociatedIds,userAssociatedIds), not a per-resource flag.Stub utilities (
filterResourcesByUserLanguages,rankByRelevance, etc.) are present in*.utils.tsfiles with sample-data-backed implementations — the API will be settled later; the point here is to communicate intent in the story.What this is NOT
ResourcePickerDialogonpt-3976-shared-resource-picker-ui— that's a dev PR; UX is upstream and ideating in parallel. Convergence comes later (per PRD Rabbit Hole Configure publishing to GH #3).Background docs
docs/plans/saroj-studies-orchestration-critique.md(lives onclaude/review-ui-prompt-H22rG/ PR docs: critique proto/saroj-studies orchestration prompt + revised v2 #2273)Test plan
cd lib/platform-bible-react && npm run storybook(port 6007). VerifyAdvanced/ModelTextZeroState,Advanced/ModelTextPicker,Advanced/ScriptureResourcesZeroState,Advanced/ScriptureResourcesTab,Advanced/ScriptureResourcesPickerrender.npm run storybook(port 6006). VerifyPlatform/NoProjectZeroStaterenders.hasLocalProjects,forceRemoveTooltipOpenForId, etc.) and confirm the documented mental model comes through.Generated by Claude Code
This change is