refactor(web): adopt <DataState> in finyk Mono panels#1703
Merged
Conversation
Replace ad-hoc `if (loadingTx && realTx.length === 0) return <Skeleton />` guards in finyk pages with the unified <DataState> wrapper: - Overview: skeleton/content branches now route through DataState; the bottom-of-page "Оновлення…" stale indicator stays untouched (still driven by loadingTx so it surfaces during background refetches). - Budgets: same pattern — skeleton extracted into a named slot, content wrapped via DataState's render prop. - TransactionList: skeleton + empty + virtualized-list paths now live inside a single DataState with isEmpty=(data) => data.length === 0. Stale-revalidate keeps the list visible thanks to the data === undefined contract (only first paint is treated as 'loading'). Adds a focused RTL test (TransactionList.test.tsx) that locks the four DataState routing branches: first-paint skeleton, post-load empty, non-empty list, and stale-while-revalidate (loading=true with prior data). Behavior-preserving migration. No hook/logic changes; presentational only. Refs: docs/initiatives/0011-foundation-adoption-and-process-discipline.md (Phase 2, PR 2.4)
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThree finyk pages (Overview, Budgets, TransactionList) transition from inline early-return loading guards to a shared ChangesDataState Adoption for Finyk Pages
Sequence DiagramsequenceDiagram
participant User
participant Page as Overview/<br/>Budgets/<br/>TransactionList
participant DataState as DataState<br/>Component
participant Skeleton as Skeleton /<br/>Empty UI
participant Content as Main<br/>Content
rect rgba(100, 150, 200, 0.5)
Note over User,Content: Initial Load (loading=true, data=undefined)
User->>Page: Render
Page->>DataState: query={data: undefined, isLoading: true}
DataState->>Skeleton: Render loading skeleton
Skeleton-->>Page: Skeleton UI visible
end
rect rgba(100, 200, 150, 0.5)
Note over User,Content: Data Arrives (loading=false, data=[...])
DataState->>Content: Render with data
Content-->>Page: Main content replaces skeleton
end
rect rgba(200, 150, 100, 0.5)
Note over User,Content: Background Refetch (loading=true, data=[...] preserved)
User->>Page: Trigger refetch
Page->>DataState: query={data: [...], isLoading: true}
DataState->>Content: Render content (data preserved)
Content-->>Page: Content stays visible, no skeleton
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/web/src/modules/finyk/pages/Overview.tsx (1)
486-488: ⚡ Quick winReplace
text-xswith a semantic text-style utility.Use the project’s
text-style-*class for this status copy to stay aligned with typography rules.As per coding guidelines, "Typography scale: use semantic
.text-style-*utilities ... and enforce 12px floor minimum for readable content."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/modules/finyk/pages/Overview.tsx` around lines 486 - 488, In the Overview component replace the presentational class on the loading status paragraph: locate the conditional that renders the <p> when loadingTx is true and swap the utility class "text-xs" for the project's semantic typography utility (e.g., "text-style-12") so the status copy uses the semantic text-style utility and respects the 12px minimum.apps/web/src/modules/finyk/pages/budgets/Budgets.tsx (1)
425-430: ⚡ Quick winUse semantic text style and explicit
focus-visiblestate on the add button.This button currently uses
text-smand no explicitfocus-visible:*styling. Please switch to the semantic typography utility and add a visible keyboard-focus style.As per coding guidelines, "Typography scale: use semantic
.text-style-*utilities" and "Visible focus indicators must usefocus-visible:".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/modules/finyk/pages/budgets/Budgets.tsx` around lines 425 - 430, The add button (the <button> that calls setShowForm(true)) uses non-semantic `text-sm` and lacks an explicit keyboard focus style; replace `text-sm` with the semantic typography utility (e.g., `text-style-sm` or the project's equivalent) and add a visible focus-visible state such as `focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary` (or your design system's focus classes) so keyboard users get a clear indicator; keep the existing hover/transition classes and class ordering consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/src/modules/finyk/pages/budgets/Budgets.tsx`:
- Around line 425-430: The add button (the <button> that calls
setShowForm(true)) uses non-semantic `text-sm` and lacks an explicit keyboard
focus style; replace `text-sm` with the semantic typography utility (e.g.,
`text-style-sm` or the project's equivalent) and add a visible focus-visible
state such as `focus-visible:outline-none focus-visible:ring-2
focus-visible:ring-primary` (or your design system's focus classes) so keyboard
users get a clear indicator; keep the existing hover/transition classes and
class ordering consistent.
In `@apps/web/src/modules/finyk/pages/Overview.tsx`:
- Around line 486-488: In the Overview component replace the presentational
class on the loading status paragraph: locate the conditional that renders the
<p> when loadingTx is true and swap the utility class "text-xs" for the
project's semantic typography utility (e.g., "text-style-12") so the status copy
uses the semantic text-style utility and respects the 12px minimum.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 64bfce0c-0298-4bc5-bebe-3d0672ed87c2
📒 Files selected for processing (4)
apps/web/src/modules/finyk/pages/Overview.tsxapps/web/src/modules/finyk/pages/budgets/Budgets.tsxapps/web/src/modules/finyk/pages/transactions/TransactionList.test.tsxapps/web/src/modules/finyk/pages/transactions/TransactionList.tsx
⏱️ CI Pipeline Duration ReportBased on the last 50 successful runs on the default branch. Overall Pipeline
Trend (last 20 runs): Per-Job Breakdown
|
This was referenced May 4, 2026
This was referenced May 4, 2026
Skords-01
pushed a commit
that referenced
this pull request
May 4, 2026
Update initiative 0011 to reflect Phase 2 progress through 2026-05-04: - Status header bumped to include the four open <DataState> consumer PRs: 2.4 #1703 (finyk), 2.5 #1709 (fizruk), 2.6 #1713 (nutrition), 2.7 #1714 (routine). Last-validated handle aligned with the actual initiative owner (@Skords-01). - Phase 2 table column 'Файли (приклад)' renamed to 'Файли (фактичні споживачі)' and ETA column replaced with Status (PR-link). Rows for 2.4–2.7 now point at the actual landed targets, not the initial guess names from the proposal draft (MonoTransactionsPanel, WorkoutHistoryPanel, NutritionMealsPanel, RoutineList, etc. — none of those components physically exist in the repo). - New explanatory note after the table records why the file list diverged from the original draft (real Skeleton-based loading sites per module) and documents the per-module finding for 2.6 / 2.7 (NutritionApp.tsx Menu plan branch; RoutineTimeline.tsx calendar branch — both modules have exactly one consumer of @shared/components/ui/Skeleton). Supersedes #1710 (single doc PR for 2.4 + 2.5 only); this combined update is the cleaner record because all four <DataState> consumer adoption PRs landed on the same day. Closes the doc-side of initiative 0011 PR 2.6 + 2.7.
9 tasks
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
1 participant
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
Адаптуємо
<DataState>wrapper у фінансових сторінках finyk: Overview, Budgets таTransactionList. Замінюємо ad-hoc патернif (loadingTx && realTx.length === 0) return <Skeleton />на єдиний presentational-врапер з контрактом «data === undefined→ skeleton;dataвизначено → content (навіть під час background refetch)».apps/web/src/modules/finyk/pages/Overview.tsx): skeleton/контент роутяться через<DataState>. Bottom-of-page «Оновлення…» індикатор лишається керованимloadingTx, тож stale-revalidate видно як раніше.apps/web/src/modules/finyk/pages/budgets/Budgets.tsx): такий самий патерн — skeleton винесено в named slot, контент огорнуто render-prop-ом<DataState>.apps/web/src/modules/finyk/pages/transactions/TransactionList.tsx): skeleton + empty + virtualized-list тепер живуть в одному<DataState>зisEmpty=(data) => data.length === 0. Stale-revalidate тримає список видимим (тільки перший paint трактується як loading).Поведінка-зберігаюча міграція. Hooks/business logic не змінюються — лише presentation. 0 active consumers
<DataState>до цього PR; це перші три.Governing Skill
sergeant-web-uisergeant-feature-delivery(для DataState contract + RTL test)Playbook
docs/playbooks/web-feature-shipping.mdVerification
Результати локально:
typecheck— pass.lint— pass (warnings лише відsergeant-design/no-hash-router-in-modulesу файлах, які я не торкав; це pre-existing для finyk/fizruk).vitest(повний suite web) — 2050 passed / 1 failed. Один failure —src/test/vercelOutputConfig.test.ts(шукає/vercel.jsonу repo root, якого немає) — pre-existing, відтворюється на чистому master і непов'язано з цим PR.TransactionList.test.tsx— 4/4 pass;DataState.test.tsx— 11/11 pass.Additional checks:
data === undefinedtriggers skeleton; persisted data +isLoading=trueтримає content і не блимає у skeleton; новий RTL test це і фіксує)Docs and Governance
AGENTS.mdneeded an update.Updated docs:
docs/initiatives/0011-foundation-adoption-and-process-discipline.mdбуде оновлено в окремому follow-up commit / PR після merge обох PR-ів (2.4 і 2.5) — там відмітимо їх як done з лінками.AGENTS.md/ playbooks / governance / review docs без змін —<DataState>API вже задокументовано вapps/web/src/shared/components/ui/DataState.tsxJSDoc +DataState.stories.tsx.Risk and Rollout
Hard Rule #15
AGENTS.mdbefore coding.--no-verify.Reviewer Notes
apps/web/src/shared/components/ui/DataState.tsxJSDoc):data === undefinedтригерить skeleton, наявнеdataтримає content навіть колиisLoading=true. У трьох місцях контракт зведено черезloadingTx && realTx.length === 0 ? undefined : realTx.TransactionListisEmptyдивиться уfiltered(post-filter), а skeleton-condition уactiveTx(pre-filter). Це навмисно: «фільтр сховав усе» ≠ «місяць порожній під час першого завантаження».loadingTxусередині render-prop-а — DataState навмисно не «з'їдає» цей UX.vercelOutputConfig.test.tsу CI — окрема історія (відсутнійvercel.jsonу repo root); не пов'язано з цим PR.