New UI #14
New UI #14
Conversation
…esigns; add progress indicators for packed items in baggage cards.
📝 WalkthroughWalkthroughAdds Google Fonts links; introduces a design system and large CSS/UI refactor; updates App header to show bag and packed counts; adds a packing progress ring to baggage cards; provides seed data generator; and adds tests for header summary and progress behavior. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/App.tsx`:
- Around line 311-315: Change the invalid block-level elements inside headings
to inline elements: replace the <div className="logo-mark"> and <div
className="logo-text-group"> used inside the <h1> (in App component) and the
loading-state header with <span> elements, keeping the same className values
(.logo-mark and .logo-text-group) so existing CSS still applies; after swapping
to <span>, ensure the CSS for .logo-mark and .logo-text-group (display:flex /
display:inline-flex and flex-direction) continues to provide the intended
layout.
🧹 Nitpick comments (6)
src/index.css (1)
69-91: Consider narrowingtransition: allto specific properties.
transition: all 0.2scan animate unintended property changes (e.g.,outline,padding, layout properties) and may trigger compositing on properties that don't need it. This applies here and throughout App.css wheretransition: allis used.Suggested change
- transition: all 0.2s; + transition: background-color 0.2s, transform 0.2s, box-shadow 0.2s;src/App.css (3)
122-124: Hardcoded colors bypass the design system.Several places in this file use hardcoded hex values instead of CSS variables, breaking the design token pattern established in
index.css. This makes future theme changes error-prone. Affected locations include:
- Line 123:
#b38c55(export hover)- Line 159:
#fdf0ed, Line 161:#f0cfc7(clear-all background/border)- Line 172:
#fbe4de(clear-all hover)- Line 309:
#fdf0ed(delete hover)- Line 390:
#3b82f6(pack-all blue)- Line 401:
rgba(59, 130, 246, 0.08)(pack-all hover)- Line 541:
#e8dfcf(item-card hover)Consider extracting these as CSS variables (e.g.,
--accent-light,--warm-hover,--info) in:rootfor consistency.
665-689: Duplicate styles for.empty-stateand.loading-state.These two rulesets (and their
svgchildren) are identical. Combine them to reduce duplication.Suggested change
-.empty-state { +.empty-state, +.loading-state { text-align: center; padding: 60px 20px; color: var(--text-muted); grid-column: 1 / -1; } -.empty-state svg { +.empty-state svg, +.loading-state svg { font-size: 64px; margin-bottom: 20px; color: var(--border); } - -.loading-state { - text-align: center; - padding: 60px 20px; - color: var(--text-muted); -} - -.loading-state svg { - font-size: 64px; - margin-bottom: 20px; - color: var(--border); -}Note:
.loading-statecurrently lacksgrid-column: 1 / -1— merging them also grants the loading state consistent grid behavior.
731-740: Consider addingprefers-reduced-motionsupport.The
fadeDownandcardUpanimations run unconditionally. Users who prefer reduced motion (accessibility setting) would benefit from having these disabled.Suggested addition after the keyframes
`@media` (prefers-reduced-motion: reduce) { .app-header, .baggage-card, .add-baggage-section { animation: none; } }src/components/BaggageCard.tsx (1)
160-171: Hardcoded stroke colors in the SVG — use CSS variables for consistency.Lines 163 and 165 hardcode
#e0d5c3and#b54830, which are the same values asvar(--border)andvar(--accent). Using inlinestylewith CSS variables keeps the ring in sync if the design tokens ever change.Suggested change
<svg className="progress-ring" viewBox="0 0 36 36"> - <circle cx="18" cy="18" r="15.9" fill="none" stroke="#e0d5c3" strokeWidth="2.5" /> - <circle cx="18" cy="18" r="15.9" fill="none" - stroke="#b54830" strokeWidth="2.5" + <circle cx="18" cy="18" r="15.9" fill="none" stroke="var(--border)" strokeWidth="2.5" /> + <circle cx="18" cy="18" r="15.9" fill="none" + stroke="var(--accent)" strokeWidth="2.5" strokeDasharray={`${pct} ${100 - pct}`} strokeDashoffset="25" strokeLinecap="round" /> </svg>SVG presentation attributes accept CSS
var()values when the SVG is inline in the DOM (as it is here), so this works without needingstyle={{}}.src/App.tsx (1)
318-323: Edge case: "0/0 packed" is shown when bags exist but contain no items.When
baggages.length > 0but none of the bags have items yet, the summary displays "0/0 packed", which isn't very informative. Consider gating the packed chip ontotalItems > 0.Suggested change
{baggages.length > 0 && ( <div className="header-summary"> <span className="summary-chip">{baggages.length} {baggages.length === 1 ? 'bag' : 'bags'}</span> - <span className="summary-chip">{totalPacked}/{totalItems} packed</span> + {totalItems > 0 && ( + <span className="summary-chip">{totalPacked}/{totalItems} packed</span> + )} </div> )}
There was a problem hiding this comment.
Pull request overview
This PR refreshes PackTracker’s UI with a new warm design system (fonts/colors/tokens), improves branding in the header, and adds at-a-glance packing progress via summaries and per-bag indicators.
Changes:
- Added a design-token-based palette and typography (Fraunces / DM Sans) and applied updated base styles.
- Updated header branding and added a header summary for bag count and packed/total items.
- Added a per-baggage circular progress indicator showing packed vs total items.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/index.css |
Introduces root design tokens and global base styles (background texture, typography, buttons). |
index.html |
Loads the new Google Fonts used by the design system. |
src/App.css |
Applies the new design system across layout/components; adds styles for header summary and progress ring. |
src/App.tsx |
Adds header summary computations and updates header markup for new branding/subtitle. |
src/components/BaggageCard.tsx |
Adds packed/total calculations and renders a progress ring per baggage card. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/test/App.test.tsx`:
- Around line 299-334: The test "updates packed count when items are
packed/unpacked" currently guards the main assertion with a conditional that
lets the test pass if no pack button is found; change this so the absence of
buttons fails the test: after querying packButtons
(document.querySelectorAll('.pack-btn') or better using
screen.getAllByRole/getAllByText), add an explicit assertion like
expect(packButtons.length).toBeGreaterThan(0) (or
expect(...).not.toHaveLength(0)) to ensure a button exists, then proceed to
await user.click(packButtons[0]) and assert the '1/2 packed' text update.
- Around line 256-297: The test currently uses conditional guards (if
(addItemBtn), if (quickAddButtons.length > 0), if (itemButtons.length > 0)) and
a swallowed promise .catch(() => {}) which lets the test pass vacuously; replace
those guards with explicit assertions and failing queries (e.g. assert the
add-item button exists instead of if (addItemBtn), assert quickAddButtons length
> 0 before proceeding, and assert itemButtons length > 0), remove the silent
.catch, and prefer getBy/getByRole or await findBy* to fail fast so missing DOM
elements cause the test to fail (look for occurrences of
document.querySelector('.add-item-btn'),
screen.queryAllByText(/T-Shirt|Pants|Shoes/i), screen.queryAllByRole('button', {
name: /T-Shirt|Pants|Shoes/i }) and the .catch(() => {}) call to update).
🧹 Nitpick comments (2)
src/data/seedData.ts (2)
12-38: Unnecessary type assertions.The string literals
'carry-on','medium-checked', and'backpack'already satisfy theBaggageTypeunion type, so theas BaggageTypecasts are redundant. Removing them lets the compiler catch typos.♻️ Remove redundant casts
- type: 'carry-on' as BaggageType, + type: 'carry-on', ... - type: 'medium-checked' as BaggageType, + type: 'medium-checked', ... - type: 'backpack' as BaggageType, + type: 'backpack',
7-8:Date.now()makes this function impure — consider accepting an optional seed parameter.If this is used in tests (the summary indicates it's a dependent of test files), non-deterministic IDs can make snapshot or equality assertions fragile. Accepting an optional
baseIdparameter would make it testable while keeping the current default behavior.♻️ Optional deterministic base ID
-export function getSeedBaggages(): Baggage[] { - const baseId = `seed-${Date.now()}`; +export function getSeedBaggages(baseId = `seed-${Date.now()}`): Baggage[] {
1 participant
This pull request introduces several UI and design improvements to the PackTracker app, focusing on enhancing the overall visual style, adding summary information, and providing better feedback on packing progress. The most notable changes include a new font and color palette, a summary display in the header, and a visual progress indicator for each baggage.
UI and Design System Updates:
:rootfor consistent styling; added a subtle diagonal stripe background texture and updated base element styles insrc/index.css. [1] [2] [3]src/App.tsx. [1] [2]Summary and Progress Indicators:
src/App.tsx.BaggageCardto visually represent the number of packed items versus the total, including a numerical display, insrc/components/BaggageCard.tsx. [1] [2]…esigns; add progress indicators for packed items in baggage cards.name: Pull Request
about: Propose changes to improve PackTracker
title: ''
labels: ''
assignees: ''
Description
Please include a summary of the change and which issue is fixed.
Also include relevant motivation and context.
Fixes # (issue)
Type of Change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist
Summary by CodeRabbit
New Features
Style
Tests