Made team page main ui + functions - #22
Conversation
|
Warning Review limit reached
Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a Teams investment browser and detail view, supporting weapon/artifact metadata, CDN helpers, a cached investment API, navigation links, homepage integration, and removal of pulls-page debug styling. ChangesTeams investment feature
Pulls debug cleanup
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TeamsPage
participant InvestmentAPI
participant InvestmentCDN
TeamsPage->>InvestmentAPI: GET /api/investment
InvestmentAPI->>InvestmentCDN: Fetch investment.json.gz
InvestmentCDN-->>InvestmentAPI: JSON or gzip bytes
InvestmentAPI-->>TeamsPage: InvestmentFile JSON
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/routes/api/investment/`+server.ts:
- Around line 17-38: Update fetchInvestment to detect gzip via the buffer’s
magic bytes before parsing, avoiding conversion of compressed data to UTF-8;
replace synchronous gunzipSync with the asynchronous gunzip API and await its
result so decompression does not block the event loop. Preserve plain JSON
parsing for non-gzipped responses and the existing InvestmentFile return
behavior.
In `@src/routes/teams/`[slug]/+page.svelte:
- Around line 110-112: Update the animation-disabled handling in the component’s
style bindings and transition definitions, including the arrow rotation, slide
transition, and portrait transitions referenced near the affected sections. When
$animationsEnabled is false, set slide duration to zero and disable all
corresponding CSS transitions; preserve the existing animated behavior when it
is true.
- Around line 230-274: Update the accordion button in the cost-group rendering
to include aria-expanded based on openCosts.has(group.cost) and aria-controls
referencing a unique panel ID for that cost group. Add the matching ID to the
conditional accordion body, ensuring each button-panel pair remains uniquely
associated.
- Around line 27-38: Update the team-loading flow around onMount and fetchTeam
so team selection reruns whenever layoutData.slug changes, not only during
initial mount. Preserve the existing fetch and error handling while ensuring the
current slug is used to select the matching team and stale team data is not
retained during navigation.
In `@src/routes/teams/`[slug]/+page.ts:
- Around line 1-5: Update the route’s load function to use the generated
PageLoad type, applying it to the function signature so params and the returned
object are type-checked against the SvelteKit route contract.
In `@src/routes/teams/`+page.svelte:
- Around line 281-285: Update the loading indicator styles in the main page
markup, including the corresponding block around the second referenced location,
to use the --pulse-animation CSS variable instead of a hard-coded animation
value. Preserve the existing loading appearance when animations are enabled and
ensure the variable’s none value disables the pulse.
- Around line 385-425: Update the character filter controls around the input
bound to inputText and the suggestions dropdown to expose combobox semantics:
provide an accessible name, set aria-expanded from showSuggestions, associate
the input with the suggestion list, and expose the active suggestion via
aria-activedescendant. Give the dropdown listbox semantics and each suggestion
button option semantics with stable IDs matching the active descendant.
- Around line 209-225: Update formatCR to render the supplied refinement value
directly for all weapons, removing the is4Star rarity-based R0 override. If
refinement level 1 must be hidden, base that condition on the refinement
argument rather than weapon rarity.
- Line 58: Update selectedCost in the teams page from null-based to
undefined-based state, including its type, reset action, and every conditional
or filter branch that checks it. Ensure the number input’s cleared value is
treated as no cost filter and preserve filtering when a numeric value is
present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f1715cf4-773e-432b-bb04-15ae15031d4f
📒 Files selected for processing (13)
src/lib/asset-urls.tssrc/lib/data/artifact-sets.jsonsrc/lib/data/artifacts.jsonsrc/lib/data/weapons.jsonsrc/lib/ui/NavBar.sveltesrc/lib/utils.tssrc/routes/+page.sveltesrc/routes/api/investment/+server.tssrc/routes/pulls/+page.sveltesrc/routes/teams/+page.sveltesrc/routes/teams/+page.tssrc/routes/teams/[slug]/+page.sveltesrc/routes/teams/[slug]/+page.ts
💤 Files with no reviewable changes (1)
- src/routes/pulls/+page.svelte
| style={!$animationsEnabled | ||
| ? "--sk-animation: none; --pulse-animation: none" | ||
| : ""} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Honor the disabled-animation preference for every transition.
The setting suppresses the entry animation, but the arrow rotation, slide transition, and portrait transitions still animate. Set the slide duration to zero and disable these CSS transitions when $animationsEnabled is false.
Also applies to: 260-274, 399-417
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/teams/`[slug]/+page.svelte around lines 110 - 112, Update the
animation-disabled handling in the component’s style bindings and transition
definitions, including the arrow rotation, slide transition, and portrait
transitions referenced near the affected sections. When $animationsEnabled is
false, set slide duration to zero and disable all corresponding CSS transitions;
preserve the existing animated behavior when it is true.
| <button | ||
| type="button" | ||
| onclick={() => toggleCost(group.cost)} | ||
| class="accordion-header flex items-center justify-between w-full px-4 py-3 text-left" | ||
| > | ||
| <div class="flex items-baseline gap-2 min-w-0"> | ||
| <span | ||
| class="text-sm font-medium" | ||
| style="color: var(--foreground-color);" | ||
| > | ||
| {group.cost} cost | ||
| </span> | ||
| <span | ||
| class="text-xs truncate hidden sm:inline" | ||
| style="color: var(--foreground-mid);" | ||
| > | ||
| — {peakSim.label} | ||
| </span> | ||
| </div> | ||
| <div class="accordion-stats"> | ||
| <span style="color: var(--foreground-mid);"> | ||
| {(peakSim.dps / 1000).toFixed(0)}K | ||
| </span> | ||
| <span style="color: var(--foreground-mid);"> | ||
| {cumulative.toFixed(1)}% | ||
| </span> | ||
| <span style="color: var(--accent-1);"> | ||
| {pctGain > 0 ? `+${pctGain.toFixed(1)}%` : ""} | ||
| </span> | ||
| <span | ||
| class="accordion-arrow transition-transform duration-200" | ||
| class:rotated={openCosts.has(group.cost)} | ||
| style="color: var(--foreground-mid);" | ||
| > | ||
| ▼ | ||
| </span> | ||
| </div> | ||
| </button> | ||
|
|
||
| <!-- Accordion body --> | ||
| {#if openCosts.has(group.cost)} | ||
| <div | ||
| class="flex flex-col" | ||
| style="border-top: 0.5px solid color-mix(in srgb, var(--accent-1) 12%, transparent);" | ||
| transition:slide={{ duration: 200 }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Expose accordion expansion state to assistive technology.
Add aria-expanded and connect each button to its panel with aria-controls/id; otherwise screen-reader users cannot determine whether a cost group is open.
Proposed fix
<button
type="button"
onclick={() => toggleCost(group.cost)}
+ aria-expanded={openCosts.has(group.cost)}
+ aria-controls={`cost-panel-${gi}`}
>
...
<div
+ id={`cost-panel-${gi}`}
class="flex flex-col"📝 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.
| <button | |
| type="button" | |
| onclick={() => toggleCost(group.cost)} | |
| class="accordion-header flex items-center justify-between w-full px-4 py-3 text-left" | |
| > | |
| <div class="flex items-baseline gap-2 min-w-0"> | |
| <span | |
| class="text-sm font-medium" | |
| style="color: var(--foreground-color);" | |
| > | |
| {group.cost} cost | |
| </span> | |
| <span | |
| class="text-xs truncate hidden sm:inline" | |
| style="color: var(--foreground-mid);" | |
| > | |
| — {peakSim.label} | |
| </span> | |
| </div> | |
| <div class="accordion-stats"> | |
| <span style="color: var(--foreground-mid);"> | |
| {(peakSim.dps / 1000).toFixed(0)}K | |
| </span> | |
| <span style="color: var(--foreground-mid);"> | |
| {cumulative.toFixed(1)}% | |
| </span> | |
| <span style="color: var(--accent-1);"> | |
| {pctGain > 0 ? `+${pctGain.toFixed(1)}%` : ""} | |
| </span> | |
| <span | |
| class="accordion-arrow transition-transform duration-200" | |
| class:rotated={openCosts.has(group.cost)} | |
| style="color: var(--foreground-mid);" | |
| > | |
| ▼ | |
| </span> | |
| </div> | |
| </button> | |
| <!-- Accordion body --> | |
| {#if openCosts.has(group.cost)} | |
| <div | |
| class="flex flex-col" | |
| style="border-top: 0.5px solid color-mix(in srgb, var(--accent-1) 12%, transparent);" | |
| transition:slide={{ duration: 200 }} | |
| <button | |
| type="button" | |
| onclick={() => toggleCost(group.cost)} | |
| aria-expanded={openCosts.has(group.cost)} | |
| aria-controls={`cost-panel-${gi}`} | |
| class="accordion-header flex items-center justify-between w-full px-4 py-3 text-left" | |
| > | |
| <div class="flex items-baseline gap-2 min-w-0"> | |
| <span | |
| class="text-sm font-medium" | |
| style="color: var(--foreground-color);" | |
| > | |
| {group.cost} cost | |
| </span> | |
| <span | |
| class="text-xs truncate hidden sm:inline" | |
| style="color: var(--foreground-mid);" | |
| > | |
| — {peakSim.label} | |
| </span> | |
| </div> | |
| <div class="accordion-stats"> | |
| <span style="color: var(--foreground-mid);"> | |
| {(peakSim.dps / 1000).toFixed(0)}K | |
| </span> | |
| <span style="color: var(--foreground-mid);"> | |
| {cumulative.toFixed(1)}% | |
| </span> | |
| <span style="color: var(--accent-1);"> | |
| {pctGain > 0 ? `+${pctGain.toFixed(1)}%` : ""} | |
| </span> | |
| <span | |
| class="accordion-arrow transition-transform duration-200" | |
| class:rotated={openCosts.has(group.cost)} | |
| style="color: var(--foreground-mid);" | |
| > | |
| ▼ | |
| </span> | |
| </div> | |
| </button> | |
| <!-- Accordion body --> | |
| {`#if` openCosts.has(group.cost)} | |
| <div | |
| id={`cost-panel-${gi}`} | |
| class="flex flex-col" | |
| style="border-top: 0.5px solid color-mix(in srgb, var(--accent-1) 12%, transparent);" | |
| transition:slide={{ duration: 200 }} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/teams/`[slug]/+page.svelte around lines 230 - 274, Update the
accordion button in the cost-group rendering to include aria-expanded based on
openCosts.has(group.cost) and aria-controls referencing a unique panel ID for
that cost group. Add the matching ID to the conditional accordion body, ensuring
each button-panel pair remains uniquely associated.
| let sortOwnedFirst = $state(true); | ||
| let sortBy = $state<"dps-desc" | "dps-asc">("dps-desc"); | ||
| /** Selected cost level — show DPS of the best sim at (or nearest to) this cost. */ | ||
| let selectedCost = $state<number | null>(null); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the cost sentinel is consistent across every use.
rg -n -C2 \
'selectedCost\s*!==\s*null|selectedCost\s*=\s*null|bind:value=\{selectedCost\}' \
src/routes/teams/+page.svelteRepository: woopxwoop/lightkeepers
Length of output: 2695
🌐 Web query:
Svelte bind:value input type=number cleared undefined null numeric binding semantics
💡 Result:
In Svelte, when using bind:value with an input element of type="number", the interaction between the UI and the bound variable follows specific semantic rules to handle non-numeric or empty states [1][2]. When the input field is cleared (becomes empty), Svelte coerces the bound variable to null [2][3][4]. This is the standard behavior designed to represent the absence of a numeric value in a way that is compatible with JavaScript and type-safe constraints, as an empty string "" cannot be represented as a valid number type [5][4]. Historically, documentation and implementation details have sometimes fluctuated regarding whether undefined or null is used for empty or invalid states, leading to some community confusion [2][6][3]. However, the current authoritative behavior, particularly emphasized in recent versions including Svelte 5, is that null is the value assigned to the bound variable when the input is empty or invalid [2][4]. Key takeaways: - Empty Input: If a user clears the input field, the bound variable is set to null [2][3]. - Invalid Input: If the input contains invalid characters (e.g., in some browsers or edge cases), the value similarly resolves to null [2][4]. - Type Consistency: This mechanism ensures that the bound variable oscillates between a number type (when the input is valid) and null (when the input is cleared or invalid), rather than becoming a string or undefined [7][4]. Developers should account for this null state in their application logic (e.g., by providing a default value or handling null checks) to avoid unexpected errors when performing arithmetic or other number-specific operations [4].
Citations:
- 1: https://svelte.dev/docs/svelte/bind
- 2: For invalid inputs in input type number, the bound value (using
bind:value={val}) is null and not undefined. sveltejs/svelte#15811 - 3: docs: fix numeric input value when string is empty to null sveltejs/svelte#9941
- 4: bug: 🐛 bind numeric inputs takes "e" as a valid input and "." leads to "null" sveltejs/svelte#8985
- 5: For input numbers we should use null instead undefined in toNumber sveltejs/svelte#1701
- 6:
input type="number"bindings can benullandundefinedsveltejs/svelte#8095 - 7: Add input validation to binded variable sveltejs/svelte#12310
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the surrounding code and the declared state type.
sed -n '50,90p' src/routes/teams/+page.svelte
printf '\n---\n'
sed -n '190,260p' src/routes/teams/+page.svelte
printf '\n---\n'
sed -n '470,740p' src/routes/teams/+page.svelteRepository: woopxwoop/lightkeepers
Length of output: 14542
🌐 Web query:
site:svelte.dev bind:value number input cleared undefined null
💡 Result:
In Svelte, the behavior of bind:value for an <input type="number"> element regarding cleared (empty) input has changed between versions: In current versions of Svelte (Svelte 5), if an input of type="number" is empty or invalid, the bound value is undefined [1][2]. In older versions of Svelte (Svelte 4 and earlier), if an input of type="number" was empty or invalid, the bound value was null [3]. If you are working with an application where you need to manage these states, it is recommended to check the specific Svelte version being used in your project [1][3][4]. For Svelte 5, you can handle the undefined state when performing calculations or data processing to avoid unexpected results [1][2].
Citations:
- 1: https://svelte.dev/docs/svelte/bind
- 2: https://svelte.dev/docs/svelte/bind/llms.txt
- 3: https://v4.svelte.dev/docs/element-directives
- 4: https://svelte.dev/docs/svelte/v5-migration-guide
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any local docs or version hints that might affect binding behavior.
git ls-files | rg '(^|/)(package\.json|svelte\.config|README\.md|docs/|src/routes/teams/\+page\.svelte)$'Repository: woopxwoop/lightkeepers
Length of output: 213
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n package.jsonRepository: woopxwoop/lightkeepers
Length of output: 1949
Use undefined for the empty cost filter sentinel.
bind:value on this number input clears to undefined in Svelte 5, so the current number | null checks keep the filter active when the field is empty. Update the state, reset action, and all selectedCost branches to use undefined.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/teams/`+page.svelte at line 58, Update selectedCost in the teams
page from null-based to undefined-based state, including its type, reset action,
and every conditional or filter branch that checks it. Ensure the number input’s
cleared value is treated as no cost filter and preserve filtering when a numeric
value is present.
**Teams list page (/teams)** - Spotlight mode as default: one team per row with tall 3:4 character portrait cards featuring element-colored backdrops and gradient overlays - Compact mode toggle: retains the original multi-column grid for power users who know exactly what they are looking for - Character portraits use CharacterIcon component, respecting user display preference (coop / TCG / enka) - Element-colored top-edge glow and radial shine hover effect on each portrait card via CSS custom properties (--shine) - Staggered slide-up entrance animations (35ms per card in compact, 50ms per row in spotlight), gated by animationsEnabled preference - Info tooltip next to TEAMS heading: click-to-toggle with gcsim/KQM attribution links, inverted styling - Combobox ARIA semantics on character tag filter: role=combobox, aria-expanded, aria-activedescendant, role=listbox/option - Spotlight pagination: 10 teams shown, "Show more" button, reset to first page on filter/sort change - Removed cost-based sort options (DPS sort only) - Docstrings on all functions **Team detail page (/teams/[slug])** - Accordion grouped by investment cost tier, closed by default - Cost header: peak DPS, cumulative % of baseline, incremental +% over previous tier, best sim label linked to config.txt - Stats aligned in CSS grid for consistent vertical scanning - Build rows (character / constellation / weapon) replacing verbose per-column detail cards - Baseline C/R badges on portrait cards - slide transition on accordion expand/collapse (200ms) - aria-expanded / aria-controls with unique panel IDs - Reactive team selection via $effect for client-side navigation **Route changes** - [slug]/+page.ts: removed SEO, typed load with PageLoad - /api/investment/+server.ts: gzip magic-byte detection, async gunzip Co-Authored-By: Claude <noreply@anthropic.com>
1 participant
Summary by CodeRabbit
New Features
Bug Fixes