Skip to content

Commit

Permalink
Merge pull request #17648 from ahmedhamidawan/fix_history_items_arrow…
Browse files Browse the repository at this point in the history 
…_nav_hid

[24.0] Fix history panel arrow navigate by id bug, add `HistoryOperations` to `HistoryView` and prevent item selection in unowned histories
  • Loading branch information
@dannon
dannon authored Mar 12, 2024
2 parents 3dbc2c6 + 84b6272 commit 35e2b98
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 41 deletions.
71 changes: 42 additions & 29 deletions client/src/components/History/Content/ContentItem.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,41 +185,54 @@ function onKeyDown(event: KeyboardEvent) {
if (event.key === "Enter" || event.key === " ") {
event.preventDefault();
onClick(event);
} else if ((event.key === "ArrowUp" || event.key === "ArrowDown") && event.shiftKey) {
event.preventDefault();
emit("shift-select", event.key);
} else if (event.key === "ArrowUp" || event.key === "ArrowDown") {
onClick();
} else if ((event.key === "ArrowUp" || event.key === "ArrowDown") && !event.shiftKey) {
event.preventDefault();
emit("init-key-selection");
emit("arrow-navigate", event.key);
} else if (event.key === "Tab") {
emit("init-key-selection");
} else if (event.key === "Delete" && !props.selected && !props.item.deleted) {
event.preventDefault();
onDelete(event.shiftKey);
} else if (event.key === "Escape") {
event.preventDefault();
emit("hide-selection");
} else if (event.key === "a" && isSelectKey(event)) {
event.preventDefault();
emit("select-all");
}
if (props.writable) {
if (event.key === "Tab") {
emit("init-key-selection");
} else {
event.preventDefault();
if ((event.key === "ArrowUp" || event.key === "ArrowDown") && event.shiftKey) {
emit("shift-select", event.key);
} else if (event.key === "ArrowUp" || event.key === "ArrowDown") {
emit("init-key-selection");
} else if (event.key === "Delete" && !props.selected && !props.item.deleted) {
onDelete(event.shiftKey);
emit("arrow-navigate", "ArrowDown");
} else if (event.key === "Escape") {
emit("hide-selection");
} else if (event.key === "a" && isSelectKey(event)) {
emit("select-all");
}
}
}
}
function onClick(e: Event) {
function onClick(e?: Event) {
const event = e as KeyboardEvent;
if (event && event.shiftKey && isSelectKey(event)) {
emit("selected-to", false);
} else if (event && isSelectKey(event)) {
emit("init-key-selection");
emit("update:selected", !props.selected);
} else if (event && event.shiftKey) {
emit("selected-to", true);
} else if (props.isPlaceholder) {
emit("init-key-selection");
} else if (props.isDataset) {
emit("init-key-selection");
if (event && props.writable) {
if (event.shiftKey && isSelectKey(event)) {
emit("selected-to", false);
return;
} else if (isSelectKey(event)) {
emit("init-key-selection");
emit("update:selected", !props.selected);
return;
} else if (event.shiftKey) {
emit("selected-to", true);
return;
} else {
emit("init-key-selection");
}
}
if (props.isPlaceholder) {
return;
}
if (props.isDataset) {
emit("update:expand-dataset", !props.expandDataset);
} else {
emit("view-collection", props.item, props.name);
Expand Down
24 changes: 14 additions & 10 deletions client/src/components/History/CurrentHistory/HistoryPanel.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ interface Props {
history: HistorySummary;
filter?: string;
canEditHistory?: boolean;
shouldShowControls?: boolean;
filterable?: boolean;
isMultiViewItem?: boolean;
}
Expand All @@ -61,7 +60,6 @@ const props = withDefaults(defineProps<Props>(), {
listOffset: 0,
filter: "",
canEditHistory: true,
shouldShowControls: true,
filterable: false,
isMultiViewItem: false,
});
Expand All @@ -80,7 +78,7 @@ const querySelectionBreak = ref(false);
const dragTarget = ref<EventTarget | null>(null);
const contentItemRefs = computed(() => {
return historyItems.value.reduce((acc: ContentItemRef, item) => {
acc[`item-${item.id}`] = ref(null);
acc[itemUniqueKey(item)] = ref(null);
return acc;
}, {});
});
Expand Down Expand Up @@ -405,6 +403,10 @@ function getItemKey(item: HistoryItem) {
return item.type_id;
}
function itemUniqueKey(item: HistoryItem) {
return `${item.history_content_type}-${item.id}`;
}
onMounted(async () => {
// `filterable` here indicates if this is the current history panel
if (props.filterable && !props.filter) {
Expand All @@ -413,9 +415,11 @@ onMounted(async () => {
await loadHistoryItems();
// if there is a listOffset, we are coming from a collection view, so focus on item at that offset
if (props.listOffset) {
const itemId = historyItems.value[props.listOffset]?.id;
const itemElement = contentItemRefs.value[`item-${itemId}`]?.value?.$el as HTMLElement;
itemElement?.focus();
const itemAtOffset = historyItems.value[props.listOffset];
if (itemAtOffset) {
const itemElement = contentItemRefs.value[itemUniqueKey(itemAtOffset)]?.value?.$el as HTMLElement;
itemElement?.focus();
}
}
});
Expand All @@ -427,7 +431,7 @@ function arrowNavigate(item: HistoryItem, eventKey: string) {
nextItem = historyItems.value[historyItems.value.indexOf(item) - 1];
}
if (nextItem) {
const itemElement = contentItemRefs.value[`item-${nextItem.id}`]?.value?.$el as HTMLElement;
const itemElement = contentItemRefs.value[itemUniqueKey(nextItem)]?.value?.$el as HTMLElement;
itemElement?.focus();
}
return nextItem;
Expand Down Expand Up @@ -510,13 +514,13 @@ function setItemDragstart(
:history="history"
:is-watching="isWatching"
:last-checked="lastCheckedTime"
:show-controls="shouldShowControls"
:show-controls="canEditHistory"
:filter-text.sync="filterText"
:hide-reload="isMultiViewItem"
@reloadContents="reloadContents" />

<HistoryOperations
v-if="shouldShowControls"
v-if="canEditHistory"
:history="history"
:show-selection="showSelection"
:expanded-count="expandedCount"
Expand Down Expand Up @@ -586,7 +590,7 @@ function setItemDragstart(
<template v-slot:item="{ item, currentOffset }">
<ContentItem
:id="item.hid"
:ref="contentItemRefs[`item-${item.id}`]"
:ref="contentItemRefs[itemUniqueKey(item)]"
is-history-item
:item="item"
:name="item.name"
Expand Down
23 changes: 22 additions & 1 deletion client/src/components/History/HistoryView.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,15 @@ describe("History center panel View", () => {
expect(tags.text()).toContain("tag_1");
expect(tags.text()).toContain("tag_2");
// HistoryCounter
expect(wrapper.find("[data-description='storage dashboard button']").attributes("disabled")).toBeTruthy();
expect(wrapper.find("[data-description='show active items button']").text()).toEqual("8");
expect(wrapper.find("[data-description='include deleted items button']").text()).toEqual("1");
expect(wrapper.find("[data-description='include hidden items button']").text()).toEqual("2");
}

function storageDashboardButtonDisabled(wrapper) {
return wrapper.find("[data-description='storage dashboard button']").attributes("disabled");
}

it("current user's current history", async () => {
const history = create_history("history_1", "user_1", false);
const wrapper = await createWrapper(localVue, "user_1", history);
Expand All @@ -117,6 +120,9 @@ describe("History center panel View", () => {
// parts of the layout that should be similar for all cases
expectCorrectLayout(wrapper);

// storage dashboard button should be enabled
expect(storageDashboardButtonDisabled(wrapper)).toBeFalsy();

// make sure all history items show up
const historyItems = wrapper.findAllComponents(ContentItem);
expect(historyItems.length).toBe(10);
Expand All @@ -138,6 +144,9 @@ describe("History center panel View", () => {
expect(switchButton.exists()).toBe(false);
expect(importButton.attributes("disabled")).toBeFalsy();

// storage dashboard button should be disabled
expect(storageDashboardButtonDisabled(wrapper)).toBeTruthy();

// parts of the layout that should be similar for all cases
expectCorrectLayout(wrapper);
});
Expand All @@ -153,6 +162,9 @@ describe("History center panel View", () => {
expect(switchButton.attributes("disabled")).toBeFalsy();
expect(importButton.exists()).toBe(false);

// storage dashboard button should be enabled
expect(storageDashboardButtonDisabled(wrapper)).toBeFalsy();

// parts of the layout that should be similar for all cases
expectCorrectLayout(wrapper);
});
Expand All @@ -168,6 +180,9 @@ describe("History center panel View", () => {
expect(switchButton.attributes("disabled")).toBeTruthy();
expect(importButton.exists()).toBe(false);

// storage dashboard button should be disabled
expect(storageDashboardButtonDisabled(wrapper)).toBeTruthy();

// instead we have an alert
expect(wrapper.find("[data-description='history state info']").text()).toBe("This history has been purged.");
});
Expand All @@ -183,6 +198,9 @@ describe("History center panel View", () => {
expect(importButton.exists()).toBe(true);
expect(importButton.attributes("disabled")).toBeFalsy();

// storage dashboard button should be disabled
expect(storageDashboardButtonDisabled(wrapper)).toBeTruthy();

expectCorrectLayout(wrapper);
expect(wrapper.find("[data-description='history state info']").exists()).toBe(false);
});
Expand All @@ -198,6 +216,9 @@ describe("History center panel View", () => {
expect(switchButton.attributes("disabled")).toBeTruthy();
expect(importButton.exists()).toBe(false);

// storage dashboard button should be disabled
expect(storageDashboardButtonDisabled(wrapper)).toBeTruthy();

expectCorrectLayout(wrapper);
expect(wrapper.find("[data-description='history state info']").text()).toBe("This history has been archived.");
});
Expand Down
1 change: 0 additions & 1 deletion client/src/components/History/HistoryView.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
v-else
:history="history"
:can-edit-history="canEditHistory"
:should-show-controls="false"
filterable
@view-collection="onViewCollection" />

Expand Down

0 comments on commit 35e2b98

Please sign in to comment.