From 4ac8777586dd02d3f495f95873e68bb7355542a3 Mon Sep 17 00:00:00 2001
From: Petr Stribny <petr@stribny.name>
Date: Sat, 4 Nov 2023 16:43:15 +0000
Subject: [PATCH] Fix shift arrow selections when primary field option wasnt
ordered first
---
...on_on_grid_view_when_primary_key_fiel.json | 7 ++
.../components/view/grid/GridView.vue | 52 +++++---
.../components/view/grid/GridViewRow.vue | 5 +-
.../modules/database/store/view/grid.js | 48 ++++---
.../unit/database/store/view/grid.spec.js | 119 ++++++++++++------
.../store/view/gridMultiSelect.spec.js | 16 +++
6 files changed, 174 insertions(+), 73 deletions(-)
create mode 100644 changelog/entries/unreleased/bug/1998_fix_shift_arrow_selection_on_grid_view_when_primary_key_fiel.json
diff --git a/changelog/entries/unreleased/bug/1998_fix_shift_arrow_selection_on_grid_view_when_primary_key_fiel.json b/changelog/entries/unreleased/bug/1998_fix_shift_arrow_selection_on_grid_view_when_primary_key_fiel.json
new file mode 100644
index 000000000..8fb5a0fd0
--- /dev/null
+++ b/changelog/entries/unreleased/bug/1998_fix_shift_arrow_selection_on_grid_view_when_primary_key_fiel.json
@@ -0,0 +1,7 @@
+{
+ "type": "bug",
+ "message": "Fix shift arrow selection on grid view when primary key field option is not first",
+ "issue_number": 1998,
+ "bullet_points": [],
+ "created_at": "2023-09-29"
+}
\ No newline at end of file
diff --git a/web-frontend/modules/database/components/view/grid/GridView.vue b/web-frontend/modules/database/components/view/grid/GridView.vue
index a60c68ef9..0b90f2a40 100644
--- a/web-frontend/modules/database/components/view/grid/GridView.vue
+++ b/web-frontend/modules/database/components/view/grid/GridView.vue
@@ -54,6 +54,7 @@
@edit-modal="openRowEditModal($event)"
@refresh-row="refreshRow"
@scroll="scroll($event.pixelY, 0)"
+ @cell-selected="cellSelected"
>
<template #foot>
<div class="grid-view__foot-info">
@@ -122,6 +123,7 @@
@edit-modal="openRowEditModal($event)"
@refresh-row="refreshRow"
@scroll="scroll($event.pixelY, $event.pixelX)"
+ @cell-selected="cellSelected"
>
</GridViewSection>
<GridViewRowDragging
@@ -368,11 +370,16 @@ export default {
...mapGetters({
row: 'rowModalNavigation/getRow',
}),
+ /**
+ * Returns all visible fields no matter in what section they
+ * belong.
+ */
allVisibleFields() {
return this.leftFields.concat(this.visibleFields)
},
/**
- * Returns only the visible fields in the correct order. Primary must always be
+ * Returns only the visible fields in the correct order that are in
+ * the right section of the grid. Primary must always be
* first if in that list.
*/
visibleFields() {
@@ -777,7 +784,7 @@ export default {
rowHeadIndex: rowIndex,
rowTailIndex: rowIndex,
fieldHeadIndex: 0,
- fieldTailIndex: this.visibleFields.length,
+ fieldTailIndex: this.allVisibleFields.length - 1,
}
)
},
@@ -1049,6 +1056,14 @@ export default {
this.$store.dispatch(this.storePrefix + 'view/grid/setSelectedCell', {
rowId: nextRowId,
fieldId: nextFieldId,
+ fields: this.fields,
+ })
+ },
+ cellSelected({ fieldId, rowId }) {
+ this.$store.dispatch(this.storePrefix + 'view/grid/setSelectedCell', {
+ rowId,
+ fieldId,
+ fields: this.fields,
})
},
/**
@@ -1070,8 +1085,7 @@ export default {
this.storePrefix + 'view/grid/multiSelectShiftClick',
{
rowId: row.id,
- fieldIndex:
- this.visibleFields.findIndex((f) => f.id === field.id) + 1,
+ fieldIndex: this.allVisibleFields.findIndex((f) => f.id === field.id),
}
)
},
@@ -1081,8 +1095,9 @@ export default {
* selected cell.
*/
multiSelectStart({ event, row, field }) {
- let fieldIndex = this.visibleFields.findIndex((f) => f.id === field.id)
- if (this.canFitInTwoColumns) fieldIndex += 1
+ const fieldIndex = this.allVisibleFields.findIndex(
+ (f) => f.id === field.id
+ )
this.$store.dispatch(this.storePrefix + 'view/grid/multiSelectStart', {
rowId: row.id,
@@ -1095,8 +1110,9 @@ export default {
* with the last cell hovered over.
*/
multiSelectHold({ event, row, field }) {
- let fieldIndex = this.visibleFields.findIndex((f) => f.id === field.id)
- if (this.canFitInTwoColumns) fieldIndex += 1
+ const fieldIndex = this.allVisibleFields.findIndex(
+ (f) => f.id === field.id
+ )
this.$store.dispatch(this.storePrefix + 'view/grid/multiSelectHold', {
rowId: row.id,
@@ -1186,20 +1202,18 @@ export default {
scrollDirection = 'vertical'
}
- const fieldId =
- this.$store.getters[this.storePrefix + 'view/grid/getFieldIdByIndex'](
- fieldIndex
- )
+ const fieldId = this.$store.getters[
+ this.storePrefix + 'view/grid/getFieldIdByIndex'
+ ](fieldIndex, this.fields)
if (fieldId === -1) {
return
}
const field = this.$store.getters['field/get'](fieldId)
const verticalContainer = this.$refs.right.$refs.body
const horizontalContainer = this.$refs.right.$el
- const visibleFieldOptions =
- this.$store.getters[
- this.storePrefix + 'view/grid/getOrderedVisibleFieldOptions'
- ]
+ const visibleFieldOptions = this.$store.getters[
+ this.storePrefix + 'view/grid/getOrderedVisibleFieldOptions'
+ ](this.fields)
let elementRight = -horizontalContainer.scrollLeft
for (let i = 0; i < visibleFieldOptions.length; i++) {
const fieldOption = visibleFieldOptions[i]
@@ -1238,6 +1252,7 @@ export default {
this.storePrefix + 'view/grid/setSelectedCellCancelledMultiSelect',
{
direction: arrowShiftKeysMapping[key],
+ fields: this.fields,
}
)
}
@@ -1280,8 +1295,9 @@ export default {
const rowIndex = this.$store.getters[
this.storePrefix + 'view/grid/getRowIndexById'
](row.id)
- let fieldIndex = this.visibleFields.findIndex((f) => f.id === field.id)
- if (this.canFitInTwoColumns) fieldIndex += 1
+ const fieldIndex = this.allVisibleFields.findIndex(
+ (f) => f.id === field.id
+ )
await this.pasteData(textData, jsonData, rowIndex, fieldIndex)
},
/**
diff --git a/web-frontend/modules/database/components/view/grid/GridViewRow.vue b/web-frontend/modules/database/components/view/grid/GridViewRow.vue
index 146947765..a7d6cd8d1 100644
--- a/web-frontend/modules/database/components/view/grid/GridViewRow.vue
+++ b/web-frontend/modules/database/components/view/grid/GridViewRow.vue
@@ -265,10 +265,7 @@ export default {
return this.row._.selected && this.row._.selectedFieldId === fieldId
},
selectCell(fieldId, rowId = this.row.id) {
- this.$store.dispatch(this.storePrefix + 'view/grid/setSelectedCell', {
- rowId,
- fieldId,
- })
+ this.$emit('cell-selected', { fieldId, rowId })
},
// Return an object that represents if a cell is selected,
// and it's current position in the selection grid
diff --git a/web-frontend/modules/database/store/view/grid.js b/web-frontend/modules/database/store/view/grid.js
index c5b81c775..1607dbb88 100644
--- a/web-frontend/modules/database/store/view/grid.js
+++ b/web-frontend/modules/database/store/view/grid.js
@@ -1222,24 +1222,26 @@ export const actions = {
setAddRowHover({ commit }, value) {
commit('SET_ADD_ROW_HOVER', value)
},
- setSelectedCell({ commit, getters, rootGetters }, { rowId, fieldId }) {
+ setSelectedCell(
+ { commit, getters, rootGetters },
+ { rowId, fieldId, fields }
+ ) {
commit('SET_SELECTED_CELL', { rowId, fieldId })
const rowIndex = getters.getRowIndexById(rowId)
if (rowIndex !== -1) {
commit('SET_MULTISELECT_START_ROW_INDEX', rowIndex)
-
- const visibleFieldEntries = getters.getOrderedVisibleFieldOptions
+ const visibleFieldOptions = getters.getOrderedVisibleFieldOptions(fields)
commit(
'SET_MULTISELECT_START_FIELD_INDEX',
- visibleFieldEntries.findIndex((f) => parseInt(f[0]) === fieldId)
+ visibleFieldOptions.findIndex((f) => parseInt(f[0]) === fieldId)
)
}
},
setSelectedCellCancelledMultiSelect(
{ commit, getters, rootGetters, dispatch },
- { direction }
+ { direction, fields }
) {
const rowIndex = getters.getMultiSelectStartRowIndex
const fieldIndex = getters.getMultiSelectStartFieldIndex
@@ -1249,7 +1251,7 @@ export const actions = {
)
const rows = getters.getAllRows
- const visibleFieldEntries = getters.getOrderedVisibleFieldOptions
+ const visibleFieldEntries = getters.getOrderedVisibleFieldOptions(fields)
const row = rows[newRowIndex - getters.getBufferStartIndex]
const field = visibleFieldEntries[newFieldIndex]
@@ -1257,6 +1259,7 @@ export const actions = {
dispatch('setSelectedCell', {
rowId: row.id,
fieldId: parseInt(field[0]),
+ fields,
})
} else {
const oldRow = rows[rowIndex - getters.getBufferStartIndex]
@@ -1266,6 +1269,7 @@ export const actions = {
dispatch('setSelectedCell', {
rowId: oldRow.id,
fieldId: parseInt(oldField[0]),
+ fields,
})
}
}
@@ -1415,7 +1419,6 @@ export const actions = {
const rowIndex = getters.getRowIndexById(rowId)
const startRowIndex = getters.getMultiSelectStartRowIndex
const startFieldIndex = getters.getMultiSelectStartFieldIndex
-
const newHeadRowIndex = Math.min(startRowIndex, rowIndex)
const newHeadFieldIndex = Math.min(startFieldIndex, fieldIndex)
const newTailRowIndex = Math.max(startRowIndex, rowIndex)
@@ -1700,6 +1703,7 @@ export const actions = {
await dispatch('setSelectedCell', {
rowId: rowsPopulated[0].id,
fieldId: primaryField.id,
+ fields,
})
}
@@ -2738,24 +2742,38 @@ export const getters = {
getAllFieldOptions(state) {
return state.fieldOptions
},
- getOrderedFieldOptions(state, getters) {
+ getOrderedFieldOptions: (state, getters) => (fields) => {
+ const primaryField = fields.find((f) => f.primary === true)
+ const primaryFieldId = primaryField?.id || -1
+
return Object.entries(getters.getAllFieldOptions)
.map(([fieldIdStr, options]) => [parseInt(fieldIdStr), options])
.sort(([a, { order: orderA }], [b, { order: orderB }]) => {
- // First by order.
+ const isAPrimary = a === primaryFieldId
+ const isBPrimary = b === primaryFieldId
+
+ // Place primary field first
+ if (isAPrimary === true && !isBPrimary) {
+ return -1
+ } else if (isBPrimary === true && !isAPrimary) {
+ return 1
+ }
+
+ // Then by order
if (orderA > orderB) {
return 1
} else if (orderA < orderB) {
return -1
}
+ // Finally by id if order is the same
return a - b
})
},
- getOrderedVisibleFieldOptions(state, getters) {
- return getters.getOrderedFieldOptions.filter(
- ([fieldId, options]) => options.hidden === false
- )
+ getOrderedVisibleFieldOptions: (state, getters) => (fields) => {
+ return getters
+ .getOrderedFieldOptions(fields)
+ .filter(([fieldId, options]) => options.hidden === false)
},
getNumberOfVisibleFields(state) {
return Object.values(state.fieldOptions).filter((fo) => fo.hidden === false)
@@ -2849,8 +2867,8 @@ export const getters = {
}
return -1
},
- getFieldIdByIndex: (state, getters) => (fieldIndex) => {
- const orderedFieldOptions = getters.getOrderedVisibleFieldOptions
+ getFieldIdByIndex: (state, getters) => (fieldIndex, fields) => {
+ const orderedFieldOptions = getters.getOrderedVisibleFieldOptions(fields)
if (orderedFieldOptions[fieldIndex]) {
return orderedFieldOptions[fieldIndex][0]
}
diff --git a/web-frontend/test/unit/database/store/view/grid.spec.js b/web-frontend/test/unit/database/store/view/grid.spec.js
index 8d44ec58a..a7a14e335 100644
--- a/web-frontend/test/unit/database/store/view/grid.spec.js
+++ b/web-frontend/test/unit/database/store/view/grid.spec.js
@@ -878,40 +878,7 @@ describe('Grid view store', () => {
})
test('getOrderedFieldOptions', () => {
- const state = Object.assign(gridStore.state(), {
- fieldOptions: {
- 1: {
- order: 0,
- hidden: false,
- },
- 2: {
- order: 1,
- hidden: true,
- },
- 3: {
- order: 3,
- hidden: false,
- },
- 4: {
- order: 2,
- hidden: false,
- },
- },
- })
- gridStore.state = () => state
- store.registerModule('grid', gridStore)
-
- expect(
- JSON.parse(JSON.stringify(store.getters['grid/getOrderedFieldOptions']))
- ).toEqual([
- [1, { hidden: false, order: 0 }],
- [2, { hidden: true, order: 1 }],
- [4, { hidden: false, order: 2 }],
- [3, { hidden: false, order: 3 }],
- ])
- })
-
- test('getOrderedVisibleFieldOptions', () => {
+ const fields = []
const state = Object.assign(gridStore.state(), {
fieldOptions: {
1: {
@@ -937,7 +904,86 @@ describe('Grid view store', () => {
expect(
JSON.parse(
- JSON.stringify(store.getters['grid/getOrderedVisibleFieldOptions'])
+ JSON.stringify(store.getters['grid/getOrderedFieldOptions'](fields))
+ )
+ ).toEqual([
+ [1, { hidden: false, order: 0 }],
+ [2, { hidden: true, order: 1 }],
+ [4, { hidden: false, order: 2 }],
+ [3, { hidden: false, order: 3 }],
+ ])
+ })
+
+ test('getOrderedFieldOptions places primary field first', () => {
+ const fields = [
+ { id: 2, primary: false },
+ { id: 3, primary: true },
+ ]
+ const state = Object.assign(gridStore.state(), {
+ fieldOptions: {
+ 1: {
+ order: 0,
+ hidden: false,
+ },
+ 2: {
+ order: 1,
+ hidden: true,
+ },
+ 3: {
+ order: 3,
+ hidden: false,
+ },
+ 4: {
+ order: 2,
+ hidden: false,
+ },
+ },
+ })
+ gridStore.state = () => state
+ store.registerModule('grid', gridStore)
+
+ expect(
+ JSON.parse(
+ JSON.stringify(store.getters['grid/getOrderedFieldOptions'](fields))
+ )
+ ).toEqual([
+ [3, { hidden: false, order: 3 }],
+ [1, { hidden: false, order: 0 }],
+ [2, { hidden: true, order: 1 }],
+ [4, { hidden: false, order: 2 }],
+ ])
+ })
+
+ test('getOrderedVisibleFieldOptions', () => {
+ const fields = []
+ const state = Object.assign(gridStore.state(), {
+ fieldOptions: {
+ 1: {
+ order: 0,
+ hidden: false,
+ },
+ 2: {
+ order: 1,
+ hidden: true,
+ },
+ 3: {
+ order: 3,
+ hidden: false,
+ },
+ 4: {
+ order: 2,
+ hidden: false,
+ },
+ },
+ })
+ gridStore.state = () => state
+ store.registerModule('grid', gridStore)
+
+ expect(
+ JSON.parse(
+ JSON.stringify(
+ store.getters['grid/getOrderedVisibleFieldOptions'](fields)
+ )
)
).toEqual([
[1, { hidden: false, order: 0 }],
@@ -979,6 +1025,7 @@ describe('Grid view store', () => {
})
test('getFieldIdByIndex', () => {
+ const fields = []
const state = Object.assign(gridStore.state(), {
fieldOptions: {
1: {
@@ -1002,6 +1049,6 @@ describe('Grid view store', () => {
gridStore.state = () => state
store.registerModule('grid', gridStore)
- expect(store.getters['grid/getFieldIdByIndex'](2)).toBe(3)
+ expect(store.getters['grid/getFieldIdByIndex'](2, fields)).toBe(3)
})
})
diff --git a/web-frontend/test/unit/database/store/view/gridMultiSelect.spec.js b/web-frontend/test/unit/database/store/view/gridMultiSelect.spec.js
index c0898bd08..56368bf4c 100644
--- a/web-frontend/test/unit/database/store/view/gridMultiSelect.spec.js
+++ b/web-frontend/test/unit/database/store/view/gridMultiSelect.spec.js
@@ -557,6 +557,7 @@ describe('Grid view multiple select', () => {
})
test('previous direction', async () => {
+ const fields = []
state.multiSelectActive = false
state.multiSelectHeadRowIndex = 14
state.multiSelectHeadFieldIndex = 1
@@ -567,6 +568,7 @@ describe('Grid view multiple select', () => {
await store.dispatch('grid/setSelectedCellCancelledMultiSelect', {
direction: 'previous',
+ fields,
})
expect(store.getters['grid/getMultiSelectHeadRowIndex']).toBe(-1)
@@ -582,6 +584,7 @@ describe('Grid view multiple select', () => {
})
test('next direction', async () => {
+ const fields = []
state.multiSelectActive = false
state.multiSelectHeadRowIndex = 14
state.multiSelectHeadFieldIndex = 1
@@ -592,6 +595,7 @@ describe('Grid view multiple select', () => {
await store.dispatch('grid/setSelectedCellCancelledMultiSelect', {
direction: 'next',
+ fields,
})
expect(store.getters['grid/getMultiSelectHeadRowIndex']).toBe(-1)
@@ -607,6 +611,7 @@ describe('Grid view multiple select', () => {
})
test('above direction', async () => {
+ const fields = []
state.multiSelectActive = false
state.multiSelectHeadRowIndex = 14
state.multiSelectHeadFieldIndex = 1
@@ -617,6 +622,7 @@ describe('Grid view multiple select', () => {
await store.dispatch('grid/setSelectedCellCancelledMultiSelect', {
direction: 'above',
+ fields,
})
expect(store.getters['grid/getMultiSelectHeadRowIndex']).toBe(-1)
@@ -632,6 +638,7 @@ describe('Grid view multiple select', () => {
})
test('below direction', async () => {
+ const fields = []
state.multiSelectActive = false
state.multiSelectHeadRowIndex = 14
state.multiSelectHeadFieldIndex = 1
@@ -642,6 +649,7 @@ describe('Grid view multiple select', () => {
await store.dispatch('grid/setSelectedCellCancelledMultiSelect', {
direction: 'below',
+ fields,
})
expect(store.getters['grid/getMultiSelectHeadRowIndex']).toBe(-1)
@@ -657,6 +665,7 @@ describe('Grid view multiple select', () => {
})
test('previous direction at the edge', async () => {
+ const fields = []
state.multiSelectActive = false
state.multiSelectHeadRowIndex = 14
state.multiSelectHeadFieldIndex = 0
@@ -667,6 +676,7 @@ describe('Grid view multiple select', () => {
await store.dispatch('grid/setSelectedCellCancelledMultiSelect', {
direction: 'previous',
+ fields,
})
expect(store.getters['grid/getMultiSelectHeadRowIndex']).toBe(-1)
@@ -682,6 +692,7 @@ describe('Grid view multiple select', () => {
})
test('next direction at the edge', async () => {
+ const fields = []
state.multiSelectActive = false
state.multiSelectHeadRowIndex = 14
state.multiSelectHeadFieldIndex = 2
@@ -692,6 +703,7 @@ describe('Grid view multiple select', () => {
await store.dispatch('grid/setSelectedCellCancelledMultiSelect', {
direction: 'next',
+ fields,
})
expect(store.getters['grid/getMultiSelectHeadRowIndex']).toBe(-1)
@@ -707,6 +719,7 @@ describe('Grid view multiple select', () => {
})
test('above direction at the edge', async () => {
+ const fields = []
state.multiSelectActive = false
state.multiSelectHeadRowIndex = 9
state.multiSelectHeadFieldIndex = 1
@@ -717,6 +730,7 @@ describe('Grid view multiple select', () => {
await store.dispatch('grid/setSelectedCellCancelledMultiSelect', {
direction: 'above',
+ fields,
})
expect(store.getters['grid/getMultiSelectHeadRowIndex']).toBe(-1)
@@ -732,6 +746,7 @@ describe('Grid view multiple select', () => {
})
test('below direction at the edge', async () => {
+ const fields = []
state.multiSelectActive = false
state.multiSelectHeadRowIndex = 17
state.multiSelectHeadFieldIndex = 1
@@ -742,6 +757,7 @@ describe('Grid view multiple select', () => {
await store.dispatch('grid/setSelectedCellCancelledMultiSelect', {
direction: 'below',
+ fields,
})
expect(store.getters['grid/getMultiSelectHeadRowIndex']).toBe(-1)