Skip to content
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.

Sign up for GitHub

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

fix(editor): Make sure auto loading and auto scrolling works in executions tab #9505

Merged
merged 3 commits into from
May 27, 2024
Merged

fix(editor): Make sure auto loading and auto scrolling works in executions tab #9505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
828226a
fix(editor): Make sure auto loading and auto scrolling works in execu…
cstuncsik May 24, 2024
5e9cb56
test: Add e2e test to check executions tab auto load and auto scroll …
cstuncsik May 24, 2024
64ad87a
fix(editor): Make auto scroll deps type
cstuncsik May 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions cypress/e2e/20-workflow-executions.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,45 @@ describe('Current Workflow Executions', () => {
cy.wait(executionsRefreshInterval);
cy.url().should('not.include', '/executions');
});

it.only('should auto load more items if there is space and auto scroll', () => {
cy.viewport(1280, 960);
executionsTab.actions.createManualExecutions(24);

cy.intercept('GET', '/rest/executions?filter=*').as('getExecutions');
cy.intercept('GET', '/rest/executions/*').as('getExecution');
executionsTab.actions.switchToExecutionsTab();

cy.wait(['@getExecutions']);
executionsTab.getters.executionListItems().its('length').should('be.gte', 10);

cy.getByTestId('current-executions-list').scrollTo('bottom');
cy.wait(['@getExecutions']);
executionsTab.getters.executionListItems().should('have.length', 24);

executionsTab.getters.executionListItems().eq(14).click();
cy.wait(['@getExecution']);
cy.reload();

cy.wait(['@getExecutions']);
executionsTab.getters.executionListItems().eq(14).should('not.be.visible');
executionsTab.getters.executionListItems().should('have.length', 24);
executionsTab.getters.executionListItems().first().should('not.be.visible');
cy.getByTestId('current-executions-list').scrollTo(0, 0);
executionsTab.getters.executionListItems().first().should('be.visible');
executionsTab.getters.executionListItems().eq(14).should('not.be.visible');

executionsTab.actions.switchToEditorTab();
executionsTab.actions.switchToExecutionsTab();

cy.wait(['@getExecutions']);
executionsTab.getters.executionListItems().eq(14).should('not.be.visible');
executionsTab.getters.executionListItems().should('have.length', 24);
executionsTab.getters.executionListItems().first().should('not.be.visible');
cy.getByTestId('current-executions-list').scrollTo(0, 0);
executionsTab.getters.executionListItems().first().should('be.visible');
executionsTab.getters.executionListItems().eq(14).should('not.be.visible');
});
});

const createMockExecutions = () => {
Expand Down
4 changes: 4 additions & 0 deletions packages/editor-ui/src/components/executions/workflow/WorkflowExecutionsCard.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ export default defineComponent({
default: false,
},
},
emits: ['retryExecution', 'mounted'],
setup() {
const executionHelpers = useExecutionHelpers();

Expand Down Expand Up @@ -147,6 +148,9 @@ export default defineComponent({
return VIEWS.EXECUTION_PREVIEW;
},
},
mounted() {
this.$emit('mounted', this.execution.id);
},
methods: {
onRetryMenuItemSelect(action: string): void {
this.$emit('retryExecution', { execution: this.execution, command: action });
Expand Down
51 changes: 41 additions & 10 deletions packages/editor-ui/src/components/executions/workflow/WorkflowExecutionsSidebar.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
:execution="execution"
:data-test-id="`execution-details-${execution.id}`"
@retry-execution="onRetryExecution"
@mounted="onItemMounted"
/>
</TransitionGroup>
<div v-if="loadingMore" class="mr-m">
Expand Down Expand Up @@ -80,6 +81,7 @@ import { useWorkflowsStore } from '@/stores/workflows.store';
import type { ExecutionFilterType } from '@/Interface';

type WorkflowExecutionsCardRef = InstanceType<typeof WorkflowExecutionsCard>;
type AutoScrollDeps = { activeExecutionSet: boolean; cardsMounted: boolean; scroll: boolean };

export default defineComponent({
name: 'WorkflowExecutionsSidebar',
Expand Down Expand Up @@ -117,6 +119,12 @@ export default defineComponent({
data() {
return {
filter: {} as ExecutionFilterType,
mountedItems: [] as string[],
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we only care about number of mounted cards here, so can we just track that instead of keeping the list of ids (I guess it will be a bit more performant for longer lists)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it could be just a counter but I need to get a workflow card ref by its ID and since it is used in other places (it is needed for the auto scroll) I didn't want to change the ref so it seemed better to collect the IDs
CleanShot 2024-05-27 at 12 19 57

I don't think it will cause a performance issue, you would have to have like 100K (or a million) of IDs stored in the array, I don't think we will ever have that

autoScrollDeps: {
activeExecutionSet: false,
cardsMounted: false,
scroll: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I find this name to be a bit unclear. Can we elaborate a bit more what this flag is doing (better name, or just a comment)?

} as AutoScrollDeps,
};
},
computed: {
Expand All @@ -129,16 +137,35 @@ export default defineComponent({
this.$router.go(-1);
}
},
},
mounted() {
// On larger screens, we need to load more then first page of executions
// for the scroll bar to appear and infinite scrolling is enabled
this.checkListSize();
setTimeout(() => {
this.scrollToActiveCard();
}, 1000);
'executionsStore.activeExecution'(
newValue: ExecutionSummary | null,
oldValue: ExecutionSummary | null,
) {
if (newValue && newValue.id !== oldValue?.id) {
this.autoScrollDeps.activeExecutionSet = true;
}
},
autoScrollDeps: {
handler(updatedDeps: AutoScrollDeps) {
if (Object.values(updatedDeps).every(Boolean)) {
this.scrollToActiveCard();
}
},
deep: true,
},
},
methods: {
onItemMounted(id: string): void {
this.mountedItems.push(id);
if (this.mountedItems.length === this.executions.length) {
this.autoScrollDeps.cardsMounted = true;
this.checkListSize();
}

if (this.executionsStore.activeExecution?.id === id) {
this.autoScrollDeps.activeExecutionSet = true;
}
},
loadMore(limit = 20): void {
if (!this.loading) {
const executionsListRef = this.$refs.executionList as HTMLElement | undefined;
Expand Down Expand Up @@ -167,7 +194,7 @@ export default defineComponent({
checkListSize(): void {
const sidebarContainerRef = this.$refs.container as HTMLElement | undefined;
const currentWorkflowExecutionsCardRefs = this.$refs[
`execution-${this.executionsStore.activeExecution?.id}`
`execution-${this.mountedItems[this.mountedItems.length - 1]}`
] as WorkflowExecutionsCardRef[] | undefined;

// Find out how many execution card can fit into list
Expand Down Expand Up @@ -196,7 +223,11 @@ export default defineComponent({
const cardRect = cardElement.getBoundingClientRect();
const LIST_HEADER_OFFSET = 200;
if (cardRect.top > executionsListRef.offsetHeight) {
executionsListRef.scrollTo({ top: cardRect.top - LIST_HEADER_OFFSET });
this.autoScrollDeps.scroll = false;
executionsListRef.scrollTo({
top: cardRect.top - LIST_HEADER_OFFSET,
behavior: 'smooth',
});
}
}
},
Expand Down
Loading