-
Notifications
You must be signed in to change notification settings - Fork 189
feat: add Create more to column creation flow #2333
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ConsoleProject ID: Sites (2)
Note You can use Avatars API to generate QR code for any text or URLs. |
WalkthroughAdds a new public boolean prop Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (2)
✨ 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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/createColumn.svelte (1)
154-159
: Confirm SideSheet boolean contract; add explicit return type.The keep-open/close behavior hinges on SideSheet consuming the boolean your
submit()
returns. Please confirm SideSheet’ssubmit.onClick
respects a resolved boolean (true = keep open, false = close). Also, make the return type explicit for clarity.Apply this diff to type the API:
-export async function submit() { +export async function submit(): Promise<boolean> {To verify SideSheet behavior, run:
#!/bin/bash # Inspect SideSheet API and footer support fd -a -i "sidesheet.svelte" | while read -r f; do echo "== $f ==" rg -n -C3 -P 'export\s+let\s+submit' "$f" rg -n -C3 -P '{#snippet\s+footer\(\)}|slot="footer"' "$f" doneAlso applies to: 165-165
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (2)
66-74
: Optional: persist the “Create more” toggle.Consider persisting
createMoreColumns
inpreferences
(keyed per table) to retain the user’s choice across sessions.
346-348
: Guard against double-submit and surface loading state on the Create button.Prevent accidental duplicate column creations and reflect loading in
disabled
.Apply this diff:
- submit={{ - text: 'Create', - onClick: async () => await createColumn?.submit(), - disabled: !selectedOption - }}> + submit={{ + text: 'Create', + onClick: async () => { + if (isCreatingColumn) return true; // keep open; debounce + isCreatingColumn = true; + try { + return await createColumn?.submit(); + } finally { + isCreatingColumn = false; + } + }, + disabled: isCreatingColumn || !selectedOption + }}>Add the flag near other locals:
let selectedOption: Option['name'] = 'String'; let createMoreColumns = false; +let isCreatingColumn = false;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
(3 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/createColumn.svelte
(3 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). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/createColumn.svelte (1)
22-23
: Prop looks good and is correctly typed/bindable.
createMore
integrates cleanly with the component API and matches the layout binding.Also applies to: 31-31
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (3)
55-55
: Import of Selector is correct.Matches usage of
<Selector.Switch />
.
360-360
: BindingcreateMore
is correct.Two-way binding aligns with the new prop and the switch state.
349-357
: Verify SideSheet supports the footer snippet.You’re using
{#snippet footer()}
; ensureSideSheet
exposes afooter
snippet/slot. If not, this block won’t render.Run:
#!/bin/bash fd -a -i "sidesheet.svelte" | xargs -I{} rg -n -C2 -P '{#snippet\s+footer\(\)}|slot="footer"' {}
...nsole)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
Show resolved
Hide resolved
...nsole)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (1)
73-77
: Resolved: reset the “create more” toggle on sheet closeThe reactive reset addresses the prior note about clearing the flag on close. Looks good.
🧹 Nitpick comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (2)
344-344
: Consider making close-on-blur conditional on the “create more” toggleSetting
closeOnBlur={false}
unconditionally makes this sheet behave differently from the others. If the goal is to prevent accidental closes only when “Create more” is enabled, gate it off the toggle:- closeOnBlur={false} + closeOnBlur={!createMoreColumns}Please confirm the intended UX relative to other SideSheets.
353-360
: Minor: add a test id for the switchHelps E2E tests target this control without brittle selectors.
- <Selector.Switch + <Selector.Switch id="create-more-columns" + data-testid="create-more-columns" bind:checked={createMoreColumns} label="Create more" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
(3 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). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (2)
364-364
: LGTM: clean two-way binding to the child propThe
bind:createMore={createMoreColumns}
wiring is straightforward and matches the new flow.
350-350
: Confirm CreateColumn.submit() return type drives SideSheet closingSideSheet’s built-in submit handler awaits your onClick value (boolean | void | Promise<boolean | void>) and closes the sheet on any falsy return. Verify that
createColumn.submit()
returnstrue
to keep it open; if it returnsvoid
orfalse
, the sheet will close. If you need explicit control, adjust the handler:- onClick: async () => await createColumn?.submit(), + onClick: async () => { + const keepOpen = await createColumn?.submit(); + if (!keepOpen) $showCreateColumnSheet.show = false; + return keepOpen; + },
What does this PR do?
Add a “create more” toggle to createColumn.svelte that can be bound.
workking same as create more in create row form
Test Plan
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
yes
Summary by CodeRabbit
New Features
Bug Fixes