mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
feat(editor): Refine the workflow review detail layout (#39866)
This commit is contained in:
Kai
GitHub
parent
834202d124
commit
373bf77f3a
|
|
@ -2,9 +2,13 @@ import type { WorkflowReviewActivityEntry } from '@n8n/api-types';
|
|||
import { createTestingPinia } from '@pinia/testing';
|
||||
import { createComponentRenderer } from '@/__tests__/render';
|
||||
import { mockedStore } from '@/__tests__/utils';
|
||||
import { computed, nextTick } from 'vue';
|
||||
import { computed, nextTick, ref } from 'vue';
|
||||
|
||||
import { ReviewLinkedWorkflowsKey, type ReviewLinkedWorkflowContext } from '../constants';
|
||||
import {
|
||||
ReviewDetailScrollContainerKey,
|
||||
ReviewLinkedWorkflowsKey,
|
||||
type ReviewLinkedWorkflowContext,
|
||||
} from '../constants';
|
||||
import { useReviewActivityStore } from '../reviewActivity.store';
|
||||
import WorkflowReviewActivityFeed from './WorkflowReviewActivityFeed.vue';
|
||||
|
||||
|
|
@ -43,6 +47,16 @@ function resizeComposer(height: number) {
|
|||
|
||||
const renderComponent = createComponentRenderer(WorkflowReviewActivityFeed);
|
||||
|
||||
/** Renders the feed with a stand-in for the detail pane's scroll area, and returns that area. */
|
||||
function renderInScrollContainer(options: Parameters<typeof renderComponent>[0] = {}) {
|
||||
const container = document.createElement('div');
|
||||
renderComponent({
|
||||
...options,
|
||||
global: { provide: { [ReviewDetailScrollContainerKey as symbol]: ref(container) } },
|
||||
});
|
||||
return container;
|
||||
}
|
||||
|
||||
type CommentEntry = Extract<WorkflowReviewActivityEntry, { type: 'comment.created' }>;
|
||||
|
||||
function makeComment(overrides: Partial<CommentEntry> = {}): WorkflowReviewActivityEntry {
|
||||
|
|
@ -228,8 +242,7 @@ describe('WorkflowReviewActivityFeed', () => {
|
|||
* differs without anything having been prepended. `scrollHeight` is stubbed because jsdom
|
||||
* has no layout; the assertion is on the feed landing at the bottom, not on any arithmetic.
|
||||
*/
|
||||
async function reloadFeedAndReturnContainer(getByTestId: (id: string) => HTMLElement) {
|
||||
const container = getByTestId('workflow-review-activity-feed');
|
||||
async function reloadFeed(container: HTMLElement) {
|
||||
Object.defineProperty(container, 'scrollHeight', { value: 500, configurable: true });
|
||||
container.scrollTop = 120;
|
||||
|
||||
|
|
@ -242,10 +255,10 @@ describe('WorkflowReviewActivityFeed', () => {
|
|||
it('shows the entry a decision just added to a feed the viewer had paged back through', async () => {
|
||||
store.entries = [makeComment({ id: '1' }), makeComment({ id: '2' })];
|
||||
|
||||
const { getByTestId } = renderComponent();
|
||||
const container = renderInScrollContainer();
|
||||
await nextTick();
|
||||
|
||||
expect((await reloadFeedAndReturnContainer(getByTestId)).scrollTop).toBe(500);
|
||||
expect((await reloadFeed(container)).scrollTop).toBe(500);
|
||||
});
|
||||
|
||||
// `loadMore` returns early while another page is in flight, leaving behind the anchor it
|
||||
|
|
@ -255,11 +268,11 @@ describe('WorkflowReviewActivityFeed', () => {
|
|||
store.hasMore = true;
|
||||
store.loadingMore = true;
|
||||
|
||||
const { getByTestId } = renderComponent();
|
||||
const container = renderInScrollContainer();
|
||||
await nextTick();
|
||||
observer.onIntersect();
|
||||
|
||||
expect((await reloadFeedAndReturnContainer(getByTestId)).scrollTop).toBe(500);
|
||||
expect((await reloadFeed(container)).scrollTop).toBe(500);
|
||||
});
|
||||
|
||||
it('does not request the same older page twice', async () => {
|
||||
|
|
@ -299,10 +312,9 @@ describe('WorkflowReviewActivityFeed', () => {
|
|||
|
||||
it('follows a growing composer only when the feed is at the bottom', async () => {
|
||||
store.entries = [makeComment({ id: '1' })];
|
||||
const { getByTestId } = renderComponent({ slots: { composer: '<textarea />' } });
|
||||
const container = renderInScrollContainer({ slots: { composer: '<textarea />' } });
|
||||
await nextTick();
|
||||
|
||||
const container = getByTestId('workflow-review-activity-feed');
|
||||
Object.defineProperty(container, 'clientHeight', { value: 300, configurable: true });
|
||||
Object.defineProperty(container, 'scrollHeight', { value: 524, configurable: true });
|
||||
|
||||
|
|
|
|||
|
|
@ -3,10 +3,11 @@ import { N8nButton, N8nLoading, N8nText } from '@n8n/design-system';
|
|||
import { useI18n } from '@n8n/i18n';
|
||||
import { useResizeObserver } from '@vueuse/core';
|
||||
import { storeToRefs } from 'pinia';
|
||||
import { onMounted, ref, watch } from 'vue';
|
||||
import { inject, onMounted, ref, watch } from 'vue';
|
||||
|
||||
import { useIntersectionObserver } from '@/app/composables/useIntersectionObserver';
|
||||
|
||||
import { ReviewDetailScrollContainerKey } from '../constants';
|
||||
import { useReviewActivityStore } from '../reviewActivity.store';
|
||||
import { resolveActivityComponent } from './activityEntryRegistry';
|
||||
|
||||
|
|
@ -14,7 +15,8 @@ const i18n = useI18n();
|
|||
const store = useReviewActivityStore();
|
||||
const { entries, loading, loadingMore, hasMore, error } = storeToRefs(store);
|
||||
|
||||
const scrollContainer = ref<HTMLElement | null>(null);
|
||||
const scrollContainer = inject(ReviewDetailScrollContainerKey, ref(null));
|
||||
const feed = ref<HTMLElement | null>(null);
|
||||
const list = ref<HTMLElement | null>(null);
|
||||
const sentinel = ref<HTMLElement | null>(null);
|
||||
const composer = ref<HTMLElement | null>(null);
|
||||
|
|
@ -109,10 +111,10 @@ useResizeObserver(composer, ([entry]) => {
|
|||
const growth = height - composerHeight;
|
||||
composerHeight = height;
|
||||
|
||||
feed.value?.style.setProperty('--review-activity--composer-height', `${height}px`);
|
||||
|
||||
const container = scrollContainer.value;
|
||||
if (!container) return;
|
||||
container.style.setProperty('--review-activity--composer-height', `${height}px`);
|
||||
if (growth <= 0) return;
|
||||
if (!container || growth <= 0) return;
|
||||
|
||||
// Was at the bottom before this growth. 1px covers rounding.
|
||||
const distanceToBottom = container.scrollHeight - container.clientHeight - container.scrollTop;
|
||||
|
|
@ -139,7 +141,7 @@ onMounted(() => {
|
|||
|
||||
<template>
|
||||
<div
|
||||
ref="scrollContainer"
|
||||
ref="feed"
|
||||
:class="[$style.feed, { [$style.feedWithComposer]: $slots.composer }]"
|
||||
data-test-id="workflow-review-activity-feed"
|
||||
>
|
||||
|
|
@ -211,11 +213,7 @@ onMounted(() => {
|
|||
</div>
|
||||
</template>
|
||||
<!-- Outside the loading and error states, so the composer stays mounted through them. -->
|
||||
<div
|
||||
v-if="$slots.composer"
|
||||
ref="composer"
|
||||
:class="[$style.composer, { [$style.composerAfterEntries]: entries.length > 0 }]"
|
||||
>
|
||||
<div v-if="$slots.composer" ref="composer" :class="$style.composer">
|
||||
<slot name="composer" />
|
||||
</div>
|
||||
</div>
|
||||
|
|
@ -235,20 +233,7 @@ onMounted(() => {
|
|||
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
flex: 1;
|
||||
min-height: 0;
|
||||
overflow: auto;
|
||||
padding-block: var(--spacing--5xs) var(--spacing--sm);
|
||||
/* Keeps the cards off the scrollbar that appears here when feed overflows */
|
||||
padding-inline-end: var(--spacing--2xs);
|
||||
}
|
||||
|
||||
/* The detail body stacks and takes over scrolling here, so the feed must bound itself or its
|
||||
load-older sentinel never leaves the screen and drains every page. */
|
||||
@container review-detail (max-width: 44rem) {
|
||||
.feed {
|
||||
max-height: 60vh;
|
||||
}
|
||||
}
|
||||
|
||||
/* Same inset the list gives its entries, so a card here starts on the avatar column. */
|
||||
|
|
@ -313,16 +298,6 @@ onMounted(() => {
|
|||
background-color: var(--color--background--light-2);
|
||||
}
|
||||
|
||||
/* Continues the timeline rail from the last entry to the composer. */
|
||||
.composerAfterEntries::before {
|
||||
content: '';
|
||||
position: absolute;
|
||||
top: var(--spacing--5xs);
|
||||
height: calc(var(--review-activity--gap) - 2 * var(--spacing--5xs));
|
||||
left: calc(var(--spacing--sm) + var(--review-activity--avatar-size) / 2);
|
||||
border-left: var(--border);
|
||||
}
|
||||
|
||||
.errorRow {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
|
|
|
|||
|
|
@ -140,6 +140,9 @@ const statusSummary = computed(() =>
|
|||
|
||||
<style module lang="scss">
|
||||
.metadata {
|
||||
position: sticky;
|
||||
top: 0;
|
||||
align-self: flex-start;
|
||||
display: flex;
|
||||
flex: 0 0 min(18rem, 30%);
|
||||
flex-direction: column;
|
||||
|
|
@ -189,6 +192,8 @@ const statusSummary = computed(() =>
|
|||
align-items: center;
|
||||
gap: var(--spacing--2xs);
|
||||
min-width: 0;
|
||||
/* Names wrap at spaces; an email breaks mid-word only when it does not fit. */
|
||||
overflow-wrap: anywhere;
|
||||
}
|
||||
|
||||
.workflow {
|
||||
|
|
@ -231,7 +236,6 @@ const statusSummary = computed(() =>
|
|||
flex-basis: auto;
|
||||
width: 100%;
|
||||
min-width: 0;
|
||||
padding-inline-end: var(--spacing--2xs);
|
||||
}
|
||||
}
|
||||
</style>
|
||||
|
|
|
|||
|
|
@ -158,7 +158,7 @@ describe('WorkflowReviewDetailTabs', () => {
|
|||
expect(getByTestId('workflow-review-no-description')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// Both go inside the feed's scroll container, so they scroll with the entries.
|
||||
// Both go inside the feed, so they scroll with the entries.
|
||||
it('places the description and the composer inside the feed', () => {
|
||||
const { getByTestId } = renderComponent({
|
||||
props: {
|
||||
|
|
|
|||
|
|
@ -2,9 +2,9 @@
|
|||
import type { WorkflowReviewInboxItem, WorkflowReviewRequestDetail } from '@n8n/api-types';
|
||||
import { N8nCallout, N8nTabs, N8nText } from '@n8n/design-system';
|
||||
import { useI18n } from '@n8n/i18n';
|
||||
import { computed, provide } from 'vue';
|
||||
import { computed, provide, useTemplateRef } from 'vue';
|
||||
|
||||
import { ReviewLinkedWorkflowsKey } from '../constants';
|
||||
import { ReviewDetailScrollContainerKey, ReviewLinkedWorkflowsKey } from '../constants';
|
||||
import type { WorkflowReviewDecisionInput } from '../workflowReviews.api';
|
||||
import WorkflowReviewActivityFeed from './WorkflowReviewActivityFeed.vue';
|
||||
import WorkflowReviewChangesSection from './WorkflowReviewChangesSection.vue';
|
||||
|
|
@ -27,6 +27,8 @@ const emit = defineEmits<{
|
|||
|
||||
const i18n = useI18n();
|
||||
|
||||
provide(ReviewDetailScrollContainerKey, useTemplateRef<HTMLElement>('detailBody'));
|
||||
|
||||
const detail = computed<WorkflowReviewRequestDetail | null>(() =>
|
||||
'workflows' in props.review ? props.review : null,
|
||||
);
|
||||
|
|
@ -122,7 +124,8 @@ const tabOptions = computed(() => [
|
|||
</div>
|
||||
</div>
|
||||
|
||||
<div :class="$style.detailBody">
|
||||
<!-- Keyed so another review or tab starts at the top. -->
|
||||
<div :key="`${review.id}:${tab}`" ref="detailBody" :class="$style.detailBody">
|
||||
<div
|
||||
v-if="tab === 'activity'"
|
||||
:class="$style.activityPanel"
|
||||
|
|
@ -229,9 +232,13 @@ const tabOptions = computed(() => [
|
|||
align-items: center;
|
||||
justify-content: space-between;
|
||||
gap: var(--spacing--sm);
|
||||
margin-right: var(--spacing--md);
|
||||
/* Holds the active-tab indicator, so the border sits under it. */
|
||||
padding-bottom: var(--review-tab-bar--indicator-overhang);
|
||||
border-bottom: var(--border);
|
||||
|
||||
> :global(.n8n-tabs) {
|
||||
transform: translateY(var(--spacing--xs));
|
||||
transform: translateY(var(--spacing--sm));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -240,24 +247,21 @@ const tabOptions = computed(() => [
|
|||
flex: 1;
|
||||
gap: var(--spacing--sm);
|
||||
min-height: 0;
|
||||
padding-top: var(--review-tab-bar--gap, calc(var(--spacing--sm) + 11px));
|
||||
overflow: auto;
|
||||
/* Inside the scroll area, so the content keeps clear of the scrollbar. */
|
||||
padding: var(--spacing--sm) var(--spacing--md) 0 0;
|
||||
}
|
||||
|
||||
.panel {
|
||||
flex: 1;
|
||||
min-height: 0;
|
||||
overflow: auto;
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
/* Separate from `.panel`: the feed brings its own scroll container. */
|
||||
.activityPanel {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
flex: 1;
|
||||
min-height: 0;
|
||||
overflow: hidden;
|
||||
max-width: var(--review-activity--max-width, 48rem);
|
||||
margin-inline-end: auto;
|
||||
min-width: 0;
|
||||
max-width: var(--review-activity--max-width, 45rem);
|
||||
margin-inline: auto;
|
||||
}
|
||||
|
||||
.descriptionCard {
|
||||
|
|
@ -297,19 +301,11 @@ const tabOptions = computed(() => [
|
|||
@container review-detail (max-width: 44rem) {
|
||||
.detailBody {
|
||||
flex-direction: column;
|
||||
overflow: auto;
|
||||
}
|
||||
|
||||
.panel {
|
||||
flex: 1 0 auto;
|
||||
overflow: visible;
|
||||
}
|
||||
|
||||
/* Auto margins would stop the stacked column from stretching. */
|
||||
.activityPanel {
|
||||
flex: 1 1 0%;
|
||||
overflow: visible;
|
||||
max-width: none;
|
||||
margin-inline-end: 0;
|
||||
margin-inline: 0;
|
||||
}
|
||||
}
|
||||
</style>
|
||||
|
|
|
|||
|
|
@ -337,11 +337,21 @@ function onListBackgroundClick() {
|
|||
}
|
||||
|
||||
.header {
|
||||
position: relative;
|
||||
display: flex;
|
||||
align-items: center;
|
||||
height: var(--review-tab-bar--height, var(--height--sm));
|
||||
padding-right: var(--spacing--md);
|
||||
margin-bottom: var(--review-tab-bar--gap, calc(var(--spacing--sm) + 11px));
|
||||
margin-bottom: var(--review-tab-bar--gap, calc(var(--spacing--sm) + 15px));
|
||||
|
||||
&::after {
|
||||
content: '';
|
||||
position: absolute;
|
||||
left: 0;
|
||||
right: var(--spacing--md);
|
||||
bottom: calc(-1 * var(--review-tab-bar--indicator-overhang) - var(--border-width));
|
||||
border-bottom: var(--border-width) solid var(--border-color);
|
||||
}
|
||||
}
|
||||
|
||||
.list {
|
||||
|
|
@ -411,10 +421,9 @@ function onListBackgroundClick() {
|
|||
padding: var(--spacing--xs);
|
||||
align-items: stretch;
|
||||
border: var(--border-width) solid var(--border-color);
|
||||
transition: background-color 0.3s ease;
|
||||
|
||||
&:hover:not(.cardSelected) {
|
||||
background-color: var(--background--active);
|
||||
background-color: var(--background--hover);
|
||||
border-color: transparent;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import type { ComputedRef, InjectionKey } from 'vue';
|
||||
import type { ComputedRef, InjectionKey, Ref } from 'vue';
|
||||
|
||||
export const WORKFLOW_REVIEW_REQUESTS_VIEW = 'WorkflowReviewRequestsView';
|
||||
|
||||
|
|
@ -20,6 +20,10 @@ export const ReviewLinkedWorkflowsKey: InjectionKey<
|
|||
ComputedRef<Map<string, ReviewLinkedWorkflowContext>>
|
||||
> = Symbol('reviewLinkedWorkflows');
|
||||
|
||||
/** The detail pane's scroll area, which the activity feed scrolls in. */
|
||||
export const ReviewDetailScrollContainerKey: InjectionKey<Readonly<Ref<HTMLElement | null>>> =
|
||||
Symbol('reviewDetailScrollContainer');
|
||||
|
||||
/**
|
||||
* Routing contract for the review inbox.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -434,11 +434,11 @@ onUnmounted(() => {
|
|||
<style lang="scss" module>
|
||||
.content {
|
||||
--review-tab-bar--height: var(--height--sm);
|
||||
--review-tab-bar--indicator-overhang: 11px;
|
||||
--review-tab-bar--indicator-overhang: 15px;
|
||||
--review-tab-bar--gap: calc(var(--spacing--sm) + var(--review-tab-bar--indicator-overhang));
|
||||
|
||||
--review-callout--max-width: 34rem;
|
||||
--review-activity--max-width: 48rem;
|
||||
--review-activity--max-width: 45rem;
|
||||
|
||||
display: flex;
|
||||
width: 100%;
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user