-
Notifications
You must be signed in to change notification settings - Fork 19
Feat/bulk action component #323
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
base: next
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR introduces a new bulk action component system that allows attaching custom Vue components to actions. The main purpose is to enable custom wrappers around action triggers, such as requiring 2FA verification before executing actions.
Key changes:
- Adds support for
customComponent
property in action definitions - Introduces
CallActionWrapper
component for handling action clicks - Updates action handling across multiple views to support custom components
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
dev-demo/resources/users.ts |
Updates user resource configuration to use new actions format and adds demo action |
dev-demo/custom/ShadowLoginButton.vue |
Adds new custom component file for testing |
adminforth/types/Back.ts |
Extends action interface to include optional customComponent property |
adminforth/spa/src/views/ShowView.vue |
Updates action button rendering to support custom components |
adminforth/spa/src/components/ThreeDotsMenu.vue |
Adds custom component support to dropdown menu actions |
adminforth/spa/src/components/ResourceListTableVirtual.vue |
Integrates custom components in virtual table action buttons |
adminforth/spa/src/components/ResourceListTable.vue |
Integrates custom components in regular table action buttons |
adminforth/spa/src/components/CallActionWrapper.vue |
New wrapper component for handling action clicks with disabled state |
adminforth/modules/configValidator.ts |
Adds validation for custom components in action definitions |
adminforth/documentation/docs/tutorial/07-Plugins/02-TwoFactorsAuth.md |
Documents how to use custom components with 2FA plugin |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
function onClick() { | ||
if (props.disabled) return; | ||
const extra = { someData: 'example' }; | ||
emit('callAction', extra); |
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.
The hardcoded 'extra' object with example data appears to be placeholder code that should be removed or made configurable.
emit('callAction', extra); | |
const props = defineProps<{ disabled?: boolean; extra?: any }>(); | |
const emit = defineEmits<{ (e: 'callAction', extra?: any ): void }>(); | |
function onClick() { | |
if (props.disabled) return; | |
emit('callAction', props.extra); |
Copilot uses AI. Check for mistakes.
</li> | ||
<li v-for="action in bulkActions.filter(a => a.showInThreeDotsDropdown)" :key="action.id"> | ||
<li v-for="action in bulkActions?.filter(a => a.showInThreeDotsDropdown)" :key="action.id"> |
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.
Adding optional chaining (?.) to bulkActions suggests it might be undefined, but this could cause silent failures. Consider checking if bulkActions exists before filtering or provide a default empty array.
<li v-for="action in bulkActions?.filter(a => a.showInThreeDotsDropdown)" :key="action.id"> | |
<li v-for="action in (bulkActions ?? []).filter(a => a.showInThreeDotsDropdown)" :key="action.id"> |
Copilot uses AI. Check for mistakes.
<button | ||
type="button" | ||
:disabled="rowActionLoadingStates?.[action.id]" | ||
@click.stop.prevent="startCustomAction(action.id, row)" |
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.
[nitpick] The click handler uses both .stop and .prevent modifiers which might be overly restrictive. Consider if both are necessary or if this could interfere with expected event behavior.
@click.stop.prevent="startCustomAction(action.id, row)" | |
@click.stop="startCustomAction(action.id, row)" |
Copilot uses AI. Check for mistakes.
<button | ||
type="button" | ||
:disabled="rowActionLoadingStates?.[action.id]" | ||
@click.stop.prevent="startCustomAction(action.id, row)" |
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.
[nitpick] The click handler uses both .stop and .prevent modifiers which might be overly restrictive. Consider if both are necessary or if this could interfere with expected event behavior.
@click.stop.prevent="startCustomAction(action.id, row)" | |
@click.prevent="startCustomAction(action.id, row)" |
Copilot uses AI. Check for mistakes.
…DotsMenu to handle null bulkActions
No description provided.