Fix: alignments, remove total table stats #2285
Conversation
ConsoleProject ID: Sites (2)
Note You can use Avatars API to generate QR code for any text or URLs. |
WalkthroughThe Container component (src/lib/layout/container.svelte) was refactored from multiple exported props to a consolidated $props() API: individual exports (expanded, slotSpacing, overlapCover, paddingInlineEnd, insideSideSheet, databasesScreen, expandHeightButton, size) were removed; new public props include children (Snippet), paddingInlineEndDouble (boolean), and databasesMainScreen (boolean). Content rendering moved from a slot to {@render children?.()}. Styling uses a derived store to compute CSS variables and arbitrary props are forwarded via restProps. New classes/CSS variants for paddingInlineEndDouble and databasesMainScreen were added. Several database-related pages and cover/header components were updated to pass databasesMainScreen and to simplify page markup (notably replacing Usage with UsageMultiple). Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/layout/container.svelte (1)
37-47: Fix inverted class binding for paddingInlineEnd
class:paddingInlineEnd={!paddingInlineEnd}applies padding when the prop is false. This likely inverts the intended semantics and can cause layout regressions (especially combined with.expanded).Apply:
- class="console-container" - class:paddingInlineEndDouble - class:paddingInlineEnd={!paddingInlineEnd}> + class="console-container" + class:paddingInlineEndDouble + class:paddingInlineEnd={paddingInlineEnd}>
🧹 Nitpick comments (2)
src/lib/layout/container.svelte (2)
30-34: Avoid empty style attribute; bind the CSS variable directlyBinding an empty string to
styleproducesstyle="". Bind the custom property directly instead.- const style = $derived( - size - ? `--p-container-max-size: var(--container-max-size, var(--container-size-${size}))` - : '' - ); + // no helper needed; bind CSS var inline on the element @@ - <div - {style} + <div + style:--p-container-max-size={size + ? `var(--container-max-size, var(--container-size-${size}))` + : undefined}Also applies to: 38-41
105-116: Ordering and overrides: expanded vs. padding classes
.expandedzeroes end padding, but.paddingInlineEnd/.paddingInlineEndDoublelater reapply padding with!important. Confirm this is intentional for expanded pages; otherwise consider removing!importantor scoping the double-padding to non-expanded variants.Also applies to: 66-83
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
src/lib/layout/container.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/settings/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/usage/[[period]]/+page.svelte(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (6)
src/lib/layout/container.svelte (1)
37-37: Verify style directive name for container-typeSvelte style directives typically use camelCase (e.g.,
style:containerType) or a staticstyleattribute. Please verifystyle:container-typecompiles across your Svelte version; if not, switch to a static style.-<div style:container-type="inline-size" class:overlap-cover={overlapCover} {...restProps}> +<div style="container-type: inline-size" class:overlap-cover={overlapCover} {...restProps}>src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.svelte (1)
21-21: LGTM: Container alignment flagsUsing
expanded slotSpacing paddingInlineEndDoublealigns with the new Container API. Re-test at ≥1024px to confirm the end padding matches adjacent UI chrome.src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte (1)
180-180: Consistency with new Container props
size="xxl"+expanded slotSpacing paddingInlineEndDoublelooks consistent. Verify that double end padding doesn’t clash with.policies-holder-cardmin-width at tablet/desktop breakpoints.src/routes/(console)/project-[region]-[project]/databases/database-[database]/settings/+page.svelte (1)
64-131: LGTM: layout-only changeSwitching to
expanded slotSpacing paddingInlineEndDoublereads well. Please spot-check the CardGrid title/aside alignment on 1024–1440px widths.src/routes/(console)/project-[region]-[project]/databases/database-[database]/usage/[[period]]/+page.svelte (2)
13-23: LGTM: simplified usage panelProps to UsageMultiple look correct (totals, counts, legend). Keep an eye on empty-state handling previously provided by the removed component.
2-2: UsageMultiple import is valid; no changes needed
src/lib/layout/index.tsexportsUsageMultiplefromusageMultiple.svelte, and there is nosrc/lib/usagedirectory.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/lib/layout/container.svelte (1)
3-5: Nice: rest props typed and children optionalTyping
...restPropsand makingchildrenoptional resolves the earlier feedback.
🧹 Nitpick comments (2)
src/lib/layout/cover.svelte (1)
125-129: Avoid width drift between Cover and Container; use a shared CSS varCover hardcodes 1200px while Container can vary. Prefer a var so both stay in sync and
sizeoverrides work consistently.- &.databasesMainScreen { - @media (min-width: 1440px) { - max-width: 1200px; - } - } + &.databasesMainScreen { + @media (min-width: 1440px) { + /* fall back to 1200px if not provided */ + max-width: var(--p-container-max-size, 1200px); + } + }src/lib/layout/container.svelte (1)
46-52: Invert-or-renamepaddingInlineEndto reduce confusion
class:paddingInlineEnd={!paddingInlineEnd}reads counterintuitively (false enables padding). Consider a clearer prop (paddingInlineEndEnabled) or inverting the binding for readability. Keep a deprecated alias for BC.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/lib/layout/container.svelte(3 hunks)src/lib/layout/cover.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/settings/+page.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/usage/[[period]]/+page.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/+page.svelte
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/settings/+page.svelte
🔇 Additional comments (6)
src/lib/layout/cover.svelte (2)
10-10: Prop addition looks good
databasesMainScreenis a clear, boolean toggle. No issues spotted.
47-47: Correct class binding
class:databasesMainScreenproperly maps the prop to styling. 👍src/routes/(console)/project-[region]-[project]/databases/database-[database]/header.svelte (1)
41-41: Tablet spacing regression checkSwitching from
expandedtodatabasesMainScreenremoves theexpanded-based top margin in Cover. Please verify header spacing on tablets (<=1024px) asmarginTopin Cover depends onexpanded.src/routes/(console)/project-[region]-[project]/databases/database-[database]/backups/+page.svelte (1)
180-180: Confirm Container width;size="xxl"currently has no effectIn the updated Container, the
sizeprop sets a CSS var that isn’t used in its CSS (see comment on container.svelte). Expect the page to render at default breakpoints unless fixed.src/routes/(console)/project-[region]-[project]/databases/database-[database]/usage/[[period]]/+page.svelte (2)
13-24: Usage UI simplification LGTMRemoving the total table usage card matches the PR goal and the remaining
UsageMultipleconfig looks consistent.
2-2: No action needed:UsageMultipleis correctly re-exported
Confirmed insrc/lib/layout/index.ts(line 14) thatUsageMultipleis exported.
What does this PR do?
Test Plan
Manual.
Related PRs and Issues
N/A.
Have you read the Contributing Guidelines on issues?
Yes.
Summary by CodeRabbit
New Features
Refactor
Style