-
Notifications
You must be signed in to change notification settings - Fork 32
feat: Add row actions to delete or duplicate a row #1713
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: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,19 +22,50 @@ | |
| :element-id="elementId" | ||
| :is-view="isView" /> | ||
| </td> | ||
| <td v-if="config.showActions" :class="{sticky: config.showActions}"> | ||
| <NcButton v-if="config.canEditRows || config.canDeleteRows" type="primary" :aria-label="t('tables', 'Edit row')" data-cy="editRowBtn" @click="$emit('edit-row', row.id)"> | ||
| <template #icon> | ||
| <Fullscreen :size="20" /> | ||
| </template> | ||
| </NcButton> | ||
| <td v-if="config.showActions" :class="{sticky: config.showActions}" class="row-actions"> | ||
| <div class="row-actions-container"> | ||
| <NcButton v-if="config.canEditRows || config.canDeleteRows" type="primary" :aria-label="t('tables', 'Edit row')" data-cy="editRowBtn" @click="$emit('edit-row', row.id)"> | ||
| <template #icon> | ||
| <Fullscreen :size="20" /> | ||
| </template> | ||
| </NcButton> | ||
| <NcActions v-if="config.canDeleteRows || config.canCreateRows" | ||
| :force-menu="true" | ||
| :aria-label="t('tables', 'Row actions')" | ||
| data-cy="tableRowActions"> | ||
| <NcActionButton v-if="config.canCreateRows" | ||
| :close-after-click="true" | ||
| data-cy="duplicateRowBtn" | ||
| @click="handleCloneRow"> | ||
| <template #icon> | ||
| <ContentCopy :size="20" /> | ||
| </template> | ||
| {{ t('tables', 'Duplicate row') }} | ||
| </NcActionButton> | ||
| <NcActionButton v-if="config.canDeleteRows" | ||
| :close-after-click="true" | ||
| data-cy="deleteRowBtn" | ||
| @click="handleDeleteRow"> | ||
| <template #icon> | ||
| <Delete :size="20" /> | ||
| </template> | ||
| {{ t('tables', 'Delete row') }} | ||
| </NcActionButton> | ||
| </NcActions> | ||
| </div> | ||
| </td> | ||
| </tr> | ||
| </template> | ||
|
|
||
| <script> | ||
| import { NcCheckboxRadioSwitch, NcButton } from '@nextcloud/vue' | ||
| import Fullscreen from 'vue-material-design-icons/Fullscreen.vue' | ||
| import { mapActions } from 'pinia' | ||
| import { useDataStore } from '../../../../store/data.js' | ||
| import { NcCheckboxRadioSwitch, NcButton, NcActions, NcActionButton } from '@nextcloud/vue' | ||
| import { showError, showSuccess } from '@nextcloud/dialogs' | ||
| import Pencil from 'vue-material-design-icons/Pencil.vue' | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pencil is unused now, I would actually vote to move from the Fullscreen to the Pencil icon for editing as that is more commonly used in Nextcloud for editing |
||
| import ContentCopy from 'vue-material-design-icons/ContentCopy.vue' | ||
| import Delete from 'vue-material-design-icons/Delete.vue' | ||
| import TableCellHtml from './TableCellHtml.vue' | ||
| import TableCellProgress from './TableCellProgress.vue' | ||
| import TableCellLink from './TableCellLink.vue' | ||
|
|
@@ -53,6 +84,7 @@ import { | |
| TYPE_META_ID, TYPE_META_CREATED_BY, TYPE_META_CREATED_AT, TYPE_META_UPDATED_BY, TYPE_META_UPDATED_AT, | ||
| } from '../../../../shared/constants.ts' | ||
| import activityMixin from '../../../mixins/activityMixin.js' | ||
| import { emit } from '@nextcloud/event-bus' | ||
|
|
||
| export default { | ||
| name: 'TableRow', | ||
|
|
@@ -65,13 +97,18 @@ export default { | |
| TableCellHtml, | ||
| NcButton, | ||
| Fullscreen, | ||
| Pencil, | ||
| ContentCopy, | ||
| Delete, | ||
| NcCheckboxRadioSwitch, | ||
| TableCellDateTime, | ||
| TableCellTextLine, | ||
| TableCellSelection, | ||
| TableCellMultiSelection, | ||
| TableCellTextRich, | ||
| TableCellUsergroup, | ||
| NcActions, | ||
| NcActionButton, | ||
| }, | ||
|
|
||
| mixins: [activityMixin], | ||
|
|
@@ -102,7 +139,7 @@ export default { | |
| }, | ||
| isView: { | ||
| type: Boolean, | ||
| default: true, | ||
| default: false, | ||
| }, | ||
| }, | ||
| computed: { | ||
|
|
@@ -197,6 +234,33 @@ export default { | |
| return text | ||
| } | ||
| }, | ||
| ...mapActions(useDataStore, ['removeRow', 'insertNewRow']), | ||
| handleDeleteRow() { | ||
| emit('tables:row:delete', { rows: [this.row.id], isView: this.isView, elementId: this.elementId }) | ||
| }, | ||
| async handleCloneRow() { | ||
| const data = this.row.data.reduce((acc, curr) => { | ||
| const column = this.visibleColumns.find(col => col.id === curr.columnId) | ||
| // Skip unique text columns to avoid constraint violations | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm unsure what the expected behaviour is. The test checks that this fails, but the logic here seems to be around skipping the values then on the newly inserted rows. However when testing this it did not skip the row. I'm fine either way, but should be clear what happens then |
||
| if (column && column.type === 'text' && column.textUnique) { | ||
| // Don't copy values from unique columns | ||
| return acc | ||
| } | ||
|
|
||
| acc[curr.columnId] = curr.value | ||
| return acc | ||
| }, {}) | ||
| const res = await this.insertNewRow({ | ||
| viewId: this.isView ? this.elementId : null, | ||
| tableId: this.isView ? null : this.elementId, | ||
| data, | ||
| }) | ||
| if (!res) { | ||
| showError(t('tables', 'Could not duplicate row.')) | ||
| } else { | ||
| showSuccess(t('tables', 'Row duplicated successfully.')) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can avoid a success message here if the new row is visible right away. |
||
| } | ||
| }, | ||
| }, | ||
| } | ||
| </script> | ||
|
|
@@ -221,5 +285,12 @@ td.fixed-width { | |
| overflow: hidden; | ||
| white-space: normal; | ||
| } | ||
| .row-actions-container { | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| gap: var(--default-grid-baseline); | ||
| min-width: calc(var(--button-size) * 2); | ||
| } | ||
|
|
||
| </style> | ||
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.
This is an unexpected change, can you explain?