feat: add flow animations to supply-chain graph edges (#154) - #165
Conversation
|
@Dev1822 is attempting to deploy a commit to the Arghya's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds a persisted animation preference, a hydration-safe settings toggle, and graph edge animation controlled by that preference with emissions-based thickness and duration. ChangesAnimation Preferences and Graph Flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsPage
participant PreferencesStore
participant SupplyChainGraph
User->>SettingsPage: Toggle Disable Animations
SettingsPage->>PreferencesStore: Persist animationsDisabled
SupplyChainGraph->>PreferencesStore: Read animationsDisabled
SupplyChainGraph->>SupplyChainGraph: Recompute animated route edges
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)package.jsonTraceback (most recent call last): 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c48450ad-10dc-4c92-b82f-f4de988771ef
📒 Files selected for processing (4)
app/[locale]/(dashboard)/settings/page.tsxcomponents/graph/supply-chain-graph.tsxhooks/index.tshooks/use-preferences.ts
📜 Review details
⚠️ CI failures not shown inline (1)
Commit Status: Vercel: Vercel
Conclusion: failure
Authorization required to deploy.
🧰 Additional context used
🪛 React Doctor (0.7.6)
app/[locale]/(dashboard)/settings/page.tsx
[warning] 74-74: Blind users can't tell what this control does because screen readers find no label, so add visible text, aria-label, or aria-labelledby.
Give every interactive control a label screen readers can read.
(control-has-associated-label)
🔇 Additional comments (1)
hooks/index.ts (1)
5-5: LGTM!
Signed-off-by: Arghyadeep Bag <arghyadeep192@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 04274925-4705-428d-8696-fb687436732a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
app/[locale]/(dashboard)/settings/page.tsxcomponents/graph/supply-chain-graph.tsxpackage.json
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: test (18.x)
- GitHub Check: test (20.x)
🧰 Additional context used
🪛 React Doctor (0.7.6)
app/[locale]/(dashboard)/settings/page.tsx
[warning] 74-74: Blind users can't tell what this control does because screen readers find no label, so add visible text, aria-label, or aria-labelledby.
Give every interactive control a label screen readers can read.
(control-has-associated-label)
🔇 Additional comments (3)
package.json (1)
63-64: LGTM!app/[locale]/(dashboard)/settings/page.tsx (1)
10-20: LGTM!Also applies to: 67-89
components/graph/supply-chain-graph.tsx (1)
28-28: LGTM!Also applies to: 79-82, 116-116, 120-138
| const thickness = emissions !== undefined ? Math.min(6, Math.max(1.5, 1.5 + (emissions / 2000))) : 1.5; | ||
| const speed = emissions !== undefined ? Math.max(0.5, 3 - (emissions / 4000)) : 2; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep animation duration within the promised 2s–0.5s range.
At zero emissions this calculates 3s, and negative emissions produce durations above 3s; only the lower bound is clamped. Normalize non-negative emissions and clamp the result to both 0.5s and 2s.
Proposed fix
- const speed = emissions !== undefined ? Math.max(0.5, 3 - (emissions / 4000)) : 2;
+ const speed =
+ emissions !== undefined
+ ? Math.min(2, Math.max(0.5, 2 - Math.max(0, emissions) / 4000))
+ : 2;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const thickness = emissions !== undefined ? Math.min(6, Math.max(1.5, 1.5 + (emissions / 2000))) : 1.5; | |
| const speed = emissions !== undefined ? Math.max(0.5, 3 - (emissions / 4000)) : 2; | |
| const thickness = emissions !== undefined ? Math.min(6, Math.max(1.5, 1.5 + (emissions / 2000))) : 1.5; | |
| const speed = | |
| emissions !== undefined | |
| ? Math.min(2, Math.max(0.5, 2 - Math.max(0, emissions) / 4000)) | |
| : 2; |
Description
This PR implements dynamic flow animations to the supply-chain graph edges to visually simulate the movement of goods and emissions through the network.
Key changes include:
animatedproperty for all connecting routes.1.5pxto6px) and animation speed (from2sto0.5s) based on the carbon volume (emissions) of each specific route.Related Issue
Closes #154
Type of Change
Checklist
npm testand all tests passnpm run lintwith no errorsSummary by CodeRabbit