Skip to content

Feat: Bulk Deletion - #485

Merged
christyjacob4 merged 9 commits into
command-center-and-migrationsfrom
bulk-deletion
Aug 9, 2023
Merged

Feat: Bulk Deletion#485
christyjacob4 merged 9 commits into
command-center-and-migrationsfrom
bulk-deletion

Conversation

@TGlide

@TGlideTGlide commented Jul 31, 2023

Copy link
Copy Markdown
Contributor

What does this PR do?

  • Adds bulk deletion to Databases, Collections and Documents

Test Plan

Manual

Related PRs and Issues

Depends on #479

Have you read the Contributing Guidelines on issues?

Yes

@TGlideTGlide added the enhancement New feature or request label Jul 31, 2023
@TGlideTGlide self-assigned this Jul 31, 2023
@vercel

vercelBot commented Jul 31, 2023•
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

NameStatusPreviewCommentsUpdated (UTC)
console❌ Failed (Inspect)Aug 8, 2023 0:26am
console-cloud❌ Failed (Inspect)Aug 8, 2023 0:26am
console-next❌ Failed (Inspect)Aug 8, 2023 0:26am

@github-actionsgithub-actionsBot mentioned this pull request Aug 1, 2023
Closed
ArmanNik
ArmanNik suggested changes Aug 4, 2023
View reviewed changes
Comment threadsrc/routes/console/project-[project]/databases/test.svelte Outdated
Comment thread...console/project-[project]/databases/database-[database]/collection-[collection]/table.svelte
Comment thread...console/project-[project]/databases/database-[database]/collection-[collection]/table.svelte Outdated
Comment thread...console/project-[project]/databases/database-[database]/collection-[collection]/table.svelte Outdated
Comment thread...console/project-[project]/databases/database-[database]/collection-[collection]/table.svelte Outdated
@TGlide
TGlide requested a review from ArmanNikAugust 7, 2023 17:42
ArmanNik
ArmanNik reviewed Aug 8, 2023
View reviewed changes
Comment thread...console/project-[project]/databases/database-[database]/collection-[collection]/table.svelte Outdated
Comment threadsrc/routes/console/project-[project]/databases/table.svelte Outdated
Comment threadsrc/routes/console/project-[project]/databases/table.svelte Outdated
@TGlide
TGlide requested a review from ArmanNikAugust 8, 2023 12:25
christyjacob4
christyjacob4 reviewed Aug 8, 2023
View reviewed changes
Comment threadsrc/lib/elements/forms/inputCheckbox.svelte
bind:checked={value}
on:invalid={handleInvalid} />
on:invalid={handleInvalid}
on:click />

@christyjacob4christyjacob4Aug 8, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't on:click have a handler ?

@TGlideTGlideAug 9, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, this forwards the event in case you need it

christyjacob4
christyjacob4 reviewed Aug 8, 2023
View reviewed changes
Comment threadsrc/lib/elements/table/tableScroll.svelte
import type { Action } from 'svelte/action';

export let isSticky = false;
export let noMargin = false;

@christyjacob4christyjacob4Aug 8, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason we use noMargin=false instead of margin=true ?
It's harder to parse through conditions when using double negatives.

@TGlideTGlideAug 9, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I do agree, this is something that's present console wise. If we change it here, we would have to change it everywhere, which is fine by me, but I'd like to run it by Torsten.

Also, if we do change it, I'd argue for hasMargin instead of just margin. Margin sounds like I'm going to pass in a numeric value

@christyjacob4christyjacob4Aug 9, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea sounds fair 👍 if this pattern is already being followed we can delay this decision for later.

christyjacob4
christyjacob4 requested changes Aug 8, 2023
View reviewed changes

@christyjacob4christyjacob4 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments. Looks good otherwise 👌

@christyjacob4
christyjacob4 merged commit 98143ef into command-center-and-migrationsAug 9, 2023
@github-actionsgithub-actionsBot mentioned this pull request Sep 1, 2023
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@christyjacob4christyjacob4christyjacob4 approved these changes
@TorstenDittmannTorstenDittmannAwaiting requested review from TorstenDittmann
+1 more reviewer
@ArmanNikArmanNikArmanNik approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@TGlideTGlide

Labels

enhancementNew feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@TGlide@christyjacob4@ArmanNik