From 077516d5e17c55b8e4e45a7c510588a97e68f665 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Tue, 14 Jan 2025 16:44:53 -0800 Subject: [PATCH 01/11] add page layout --- .../src/components/ui/tab-group/tab-group.ts | 40 +++++++------------ frontend/src/layouts/pageSectionsWithNav.ts | 36 +++++++++++++++++ 2 files changed, 51 insertions(+), 25 deletions(-) create mode 100644 frontend/src/layouts/pageSectionsWithNav.ts diff --git a/frontend/src/components/ui/tab-group/tab-group.ts b/frontend/src/components/ui/tab-group/tab-group.ts index 076218b3d3..fe34449e9b 100644 --- a/frontend/src/components/ui/tab-group/tab-group.ts +++ b/frontend/src/components/ui/tab-group/tab-group.ts @@ -1,4 +1,3 @@ -import clsx from "clsx"; import { html, type PropertyValues } from "lit"; import { customElement, @@ -10,7 +9,7 @@ import type { TabClickDetail, TabGroupTab } from "./tab"; import { type TabGroupPanel } from "./tab-panel"; import { TailwindElement } from "@/classes/TailwindElement"; -import { tw } from "@/utils/tailwind"; +import { pageSectionsWithNav } from "@/layouts/pageSectionsWithNav"; /** * @example Usage: @@ -33,6 +32,10 @@ export class TabGroup extends TailwindElement { @property({ type: String }) placement: "top" | "start" = "top"; + /* Nav sticky */ + @property({ type: Boolean }) + sticky = true; + @property({ type: String, noAccessor: true, reflect: true }) role = "tablist"; @@ -61,29 +64,16 @@ export class TabGroup extends TailwindElement { } render() { - return html` -
-
- -
-
- -
-
- `; + return pageSectionsWithNav({ + nav: html``, + main: html``, + placement: this.placement, + sticky: this.sticky, + }); } private handleActiveChange() { diff --git a/frontend/src/layouts/pageSectionsWithNav.ts b/frontend/src/layouts/pageSectionsWithNav.ts new file mode 100644 index 0000000000..44c7db903f --- /dev/null +++ b/frontend/src/layouts/pageSectionsWithNav.ts @@ -0,0 +1,36 @@ +import clsx from "clsx"; +import { html, type TemplateResult } from "lit"; + +import { tw } from "@/utils/tailwind"; + +export function pageSectionsWithNav({ + nav, + main, + placement = "start", + sticky = false, +}: { + nav: TemplateResult; + main: TemplateResult; + placement?: "start" | "top"; + sticky?: boolean; +}) { + return html` +
+
+ ${nav} +
+
${main}
+
+ `; +} From 0ec55550b9e9b1be8b0808abfa36db481797d038 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Tue, 14 Jan 2025 17:53:24 -0800 Subject: [PATCH 02/11] add form sections --- .../crawl-workflows/workflow-editor.ts | 370 +++++------------- frontend/src/layouts/panel.ts | 39 ++ 2 files changed, 140 insertions(+), 269 deletions(-) create mode 100644 frontend/src/layouts/panel.ts diff --git a/frontend/src/features/crawl-workflows/workflow-editor.ts b/frontend/src/features/crawl-workflows/workflow-editor.ts index d0023fb506..2933c09a1f 100644 --- a/frontend/src/features/crawl-workflows/workflow-editor.ts +++ b/frontend/src/features/crawl-workflows/workflow-editor.ts @@ -1,13 +1,11 @@ import { consume } from "@lit/context"; import { localized, msg, str } from "@lit/localize"; import type { - SlChangeEvent, SlCheckbox, SlInput, SlRadio, SlRadioGroup, SlSelect, - SlSwitch, SlTextarea, } from "@shoelace-style/shoelace"; import Fuse from "fuse.js"; @@ -50,6 +48,8 @@ import { type SelectBrowserProfileChangeEvent } from "@/features/browser-profile import type { CollectionsChangeEvent } from "@/features/collections/collections-add"; import type { QueueExclusionTable } from "@/features/crawl-workflows/queue-exclusion-table"; import { infoCol, inputCol } from "@/layouts/columns"; +import { pageSectionsWithNav } from "@/layouts/pageSectionsWithNav"; +import { panel } from "@/layouts/panel"; import infoTextStrings from "@/strings/crawl-workflows/infoText"; import scopeTypeLabels from "@/strings/crawl-workflows/scopeType"; import sectionStrings from "@/strings/crawl-workflows/section"; @@ -231,6 +231,15 @@ export class WorkflowEditor extends BtrixElement { private readonly validateNameMax = maxLengthValidator(50); private readonly validateDescriptionMax = maxLengthValidator(350); + private readonly tabLabels: Record = { + crawlSetup: sectionStrings.scope, + crawlLimits: msg("Limits"), + browserSettings: sectionStrings.browserSettings, + crawlScheduling: sectionStrings.scheduling, + crawlMetadata: msg("Metadata"), + confirmSettings: msg("Review Settings"), + }; + private get formHasError() { return ( !this.hasRequiredFields() || @@ -378,21 +387,6 @@ export class WorkflowEditor extends BtrixElement { } render() { - const tabLabels: Record = { - crawlSetup: sectionStrings.scope, - crawlLimits: msg("Limits"), - browserSettings: sectionStrings.browserSettings, - crawlScheduling: sectionStrings.scheduling, - crawlMetadata: msg("Metadata"), - confirmSettings: msg("Review Settings"), - }; - let orderedTabNames = STEPS as readonly StepName[]; - - if (this.configId) { - // Remove review tab - orderedTabNames = orderedTabNames.slice(0, -1); - } - return html`
- -
-

- ${tabLabels[this.progressState!.activeTab]} -

-

- ${msg( - html`Fields marked with - * - are required`, - )} -

-
- - ${orderedTabNames.map((tabName) => - this.renderNavItem(tabName, tabLabels[tabName]), - )} - - - ${this.renderPanelContent(this.renderScope(), { - isFirst: true, - })} - - - ${this.renderPanelContent(this.renderCrawlLimits())} - - - ${this.renderPanelContent(this.renderCrawlBehaviors())} - - - ${this.renderPanelContent(this.renderJobScheduling())} - - - ${this.renderPanelContent(this.renderJobMetadata())} - - - ${this.renderPanelContent(this.renderConfirmSettings(), { - isLast: true, - })} - - + ${pageSectionsWithNav({ + nav: this.renderNav(), + main: this.renderFormSections(), + sticky: true, + })} + ${this.renderFooter()}
`; } - private renderNavItem(tabName: StepName, content: TemplateResult | string) { - const isActive = tabName === this.progressState!.activeTab; - const isConfirmSettings = tabName === "confirmSettings"; - const { error: isInvalid, completed } = this.progressState!.tabs[tabName]; - let icon: TemplateResult = html``; - - if (!this.configId) { - const iconProps = { - name: "circle", - library: "default", - class: "text-neutral-400", - }; - if (isConfirmSettings) { - iconProps.name = "info-circle"; - iconProps.class = "text-base"; - } else { - if (isInvalid) { - iconProps.name = "exclamation-circle"; - iconProps.class = "text-danger"; - } else if (isActive) { - iconProps.name = "pencil-circle-dashed"; - iconProps.library = "app"; - iconProps.class = "text-base"; - } else if (completed) { - iconProps.name = "check-circle"; - } - } + private renderNav() { + let orderedTabNames = STEPS as readonly StepName[]; - icon = html` - - - - `; + if (this.configId) { + // Remove review tab + orderedTabNames = orderedTabNames.slice(0, -1); } return html` - - ${icon} - - ${content} - - - `; - } - - private renderPanelContent( - content: TemplateResult, - { isFirst = false, isLast = false } = {}, - ) { - return html` -
-
- ${content} - ${when(this.serverError, () => - this.renderErrorAlert(this.serverError!), - )} -
- - ${this.renderFooter({ isFirst, isLast })} -
+ ${orderedTabNames.map( + (tab) => html` + + ${this.tabLabels[tab]} + + `, + )} `; } - private renderFooter({ isFirst = false, isLast = false }) { - if (this.configId) { - return html` -
-
${this.renderRunNowToggle()}
- - - ${msg("Save Workflow")} - -
- `; - } - - if (!this.configId) { - return html` -
- ${this.renderSteppedFooterButtons({ isFirst, isLast })} -
- `; - } - + private renderFormSections() { return html` -
- ${when( - this.configId, - () => html` -
${this.renderRunNowToggle()}
- + ${this.formSections.map(({ name, desc, render, open }) => + panel({ + heading: this.tabLabels[name], + content: html` - ${msg("Save Changes")} - - `, - () => this.renderSteppedFooterButtons({ isFirst, isLast }), +

${desc}

+
${render.bind(this)()}
+ `, + }), )}
+ + ${when(this.serverError, (error) => this.renderErrorAlert(error))} `; } - private renderSteppedFooterButtons({ - isFirst, - isLast, - }: { - isFirst: boolean; - isLast: boolean; - }) { - if (isLast) { - return html` + + ${msg("Cancel")} + + - - ${msg("Previous Step")} + ${this.configId ? msg("Save") : msg("Create")} - ${this.renderRunNowToggle()} - ${msg("Save Workflow")} - `; - } - return html` - ${isFirst - ? nothing - : html` - - - ${msg("Previous Step")} - - `} - - - ${msg("Next Step")} - - { - if (this.hasRequiredFields()) { - this.updateProgressState({ - activeTab: "confirmSettings", - }); - } else { - this.nextStep(); - } - }} - > - - ${msg(html`Review & Save`)} - - `; - } - - private renderRunNowToggle() { - return html` - { - this.updateFormState( - { - runNow: (e.target as SlSwitch).checked, - }, - true, - ); - }} - > - ${msg("Run on Save")} - + ${this.configId ? msg("Save and Run") : msg("Run Workflow")} + + `; } @@ -1660,7 +1456,7 @@ https://archiveweb.page/images/${"logo.svg"}`} private renderErrorAlert(errorMessage: string | TemplateResult) { return html` -
+
${errorMessage}
`; @@ -1718,6 +1514,42 @@ https://archiveweb.page/images/${"logo.svg"}`} `; }; + private readonly formSections: { + name: StepName; + desc: string; + render: () => TemplateResult<1>; + open?: boolean; + }[] = [ + { + name: "crawlSetup", + desc: msg("Specify the range and depth of your crawl."), + render: this.renderScope, + open: true, + }, + { + name: "crawlLimits", + desc: msg("Enforce maximum limits on your crawl."), + render: this.renderCrawlLimits, + }, + { + name: "browserSettings", + desc: msg( + "Configure the browser that's used to visit URLs during the crawl.", + ), + render: this.renderCrawlBehaviors, + }, + { + name: "crawlScheduling", + desc: msg("Schedule recurring crawls."), + render: this.renderJobScheduling, + }, + { + name: "crawlMetadata", + desc: msg("Describe and organize crawls from this workflow."), + render: this.renderJobMetadata, + }, + ]; + private changeScopeType(value: FormState["scopeType"]) { const prevScopeType = this.formState.scopeType; const formState: Partial = { diff --git a/frontend/src/layouts/panel.ts b/frontend/src/layouts/panel.ts new file mode 100644 index 0000000000..0d292d5fa8 --- /dev/null +++ b/frontend/src/layouts/panel.ts @@ -0,0 +1,39 @@ +import { html, type TemplateResult } from "lit"; + +import { pageHeading } from "./page"; + +export function panelHeader({ + heading, + actions, +}: { + heading: string | Parameters[0]; + actions?: TemplateResult; +}) { + return html` +
+ ${typeof heading === "string" + ? pageHeading({ content: heading }) + : pageHeading(heading)} + ${actions} +
+ `; +} + +export function panelBody({ content }: { content: TemplateResult }) { + return html`
${content}
`; +} + +/** + * @TODO Refactor components to use panel + */ +export function panel({ + content, + heading, + actions, +}: { + content: TemplateResult; +} & Parameters[0]) { + return html`
+ ${panelHeader({ heading, actions })} ${content} +
`; +} From 484efc3713171b0c4144e4689c6e65849353970f Mon Sep 17 00:00:00 2001 From: sua yoo Date: Tue, 14 Jan 2025 18:03:47 -0800 Subject: [PATCH 03/11] handle required fields --- .../crawl-workflows/workflow-editor.ts | 63 ++++++++++++------- 1 file changed, 42 insertions(+), 21 deletions(-) diff --git a/frontend/src/features/crawl-workflows/workflow-editor.ts b/frontend/src/features/crawl-workflows/workflow-editor.ts index 2933c09a1f..e0096a6a42 100644 --- a/frontend/src/features/crawl-workflows/workflow-editor.ts +++ b/frontend/src/features/crawl-workflows/workflow-editor.ts @@ -428,16 +428,27 @@ export class WorkflowEditor extends BtrixElement { private renderFormSections() { return html`
- ${this.formSections.map(({ name, desc, render, open }) => + ${this.formSections.map(({ name, desc, render, required }) => panel({ heading: this.tabLabels[name], content: html`

${desc}

${render.bind(this)()}
`, + actions: required + ? html`

+ ${msg( + html`Fields marked with + * + are required`, + )} +

` + : undefined, }), )}
@@ -447,6 +458,8 @@ export class WorkflowEditor extends BtrixElement { } private renderFooter() { + const hasRequiredFields = this.hasRequiredFields(); + return html`
${msg("Cancel")} - - ${this.configId ? msg("Save") : msg("Create")} - - - ${this.configId ? msg("Save and Run") : msg("Run Workflow")} - + + ${this.configId ? msg("Save") : msg("Create")} + + + ${this.configId ? msg("Save and Run") : msg("Run Workflow")} + +
`; } @@ -1518,13 +1539,13 @@ https://archiveweb.page/images/${"logo.svg"}`} name: StepName; desc: string; render: () => TemplateResult<1>; - open?: boolean; + required?: boolean; }[] = [ { name: "crawlSetup", desc: msg("Specify the range and depth of your crawl."), render: this.renderScope, - open: true, + required: true, }, { name: "crawlLimits", From 8cc5abeb3971060faa3fb0d2550fc6a7074cb34d Mon Sep 17 00:00:00 2001 From: sua yoo Date: Tue, 14 Jan 2025 18:48:46 -0800 Subject: [PATCH 04/11] update sections --- .../crawl-workflows/workflow-editor.ts | 239 +++++++----------- frontend/src/layouts/panel.ts | 7 +- 2 files changed, 99 insertions(+), 147 deletions(-) diff --git a/frontend/src/features/crawl-workflows/workflow-editor.ts b/frontend/src/features/crawl-workflows/workflow-editor.ts index e0096a6a42..6a7251b68c 100644 --- a/frontend/src/features/crawl-workflows/workflow-editor.ts +++ b/frontend/src/features/crawl-workflows/workflow-editor.ts @@ -2,12 +2,14 @@ import { consume } from "@lit/context"; import { localized, msg, str } from "@lit/localize"; import type { SlCheckbox, + SlHideEvent, SlInput, SlRadio, SlRadioGroup, SlSelect, SlTextarea, } from "@shoelace-style/shoelace"; +import clsx from "clsx"; import Fuse from "fuse.js"; import { mergeDeep } from "immutable"; import type { LanguageCode } from "iso-639-1"; @@ -53,12 +55,7 @@ import { panel } from "@/layouts/panel"; import infoTextStrings from "@/strings/crawl-workflows/infoText"; import scopeTypeLabels from "@/strings/crawl-workflows/scopeType"; import sectionStrings from "@/strings/crawl-workflows/section"; -import { - ScopeType, - type CrawlConfig, - type Seed, - type WorkflowParams, -} from "@/types/crawler"; +import { ScopeType, type Seed, type WorkflowParams } from "@/types/crawler"; import { NewWorkflowOnlyScopeType } from "@/types/workflow"; import { isApiError, type Detail } from "@/utils/api"; import { DEPTH_SUPPORTED_SCOPES, isPageScopeType } from "@/utils/crawler"; @@ -97,7 +94,6 @@ const STEPS = [ "browserSettings", "crawlScheduling", "crawlMetadata", - "confirmSettings", ] as const; type StepName = (typeof STEPS)[number]; type TabState = { @@ -115,6 +111,7 @@ const DEFAULT_BEHAVIORS = [ "autofetch", "siteSpecific", ]; +const formName = "newJobConfig" as const; const getDefaultProgressState = (hasConfigId = false): ProgressState => { let activeTab: StepName = "crawlSetup"; @@ -146,10 +143,6 @@ const getDefaultProgressState = (hasConfigId = false): ProgressState => { error: false, completed: hasConfigId, }, - confirmSettings: { - error: false, - completed: hasConfigId, - }, }, }; }; @@ -237,7 +230,6 @@ export class WorkflowEditor extends BtrixElement { browserSettings: sectionStrings.browserSettings, crawlScheduling: sectionStrings.scheduling, crawlMetadata: msg("Metadata"), - confirmSettings: msg("Review Settings"), }; private get formHasError() { @@ -280,11 +272,11 @@ export class WorkflowEditor extends BtrixElement { "": "", }; - @query('form[name="newJobConfig"]') - formElem?: HTMLFormElement; + @query(`form[name="${formName}"]`) + private readonly formElem?: HTMLFormElement; - @queryAsync("btrix-tab-panel[aria-hidden=false]") - activeTabPanel!: Promise; + @queryAsync(`.${formName}--panel--active`) + private readonly activeTabPanel!: Promise; connectedCallback(): void { this.initializeEditor(); @@ -389,7 +381,7 @@ export class WorkflowEditor extends BtrixElement { render() { return html`
html` - - ${this.tabLabels[tab]} - - `, - )} - `; + const button = (tab: StepName) => { + const isActive = tab === this.progressState?.activeTab; + return html` + + ${this.tabLabels[tab]} + + `; + }; + + return html` ${orderedTabNames.map(button)} `; } private renderFormSections() { + const content = ({ + name, + desc, + render, + required, + }: (typeof this.formSections)[number]) => { + const hasError = this.progressState?.tabs[name].error; + + return html` { + if (hasError) { + e.preventDefault(); + // TODO focus on error + } + }} + > +

${desc}

+
${render.bind(this)()}
+ `; + }; return html`
- ${this.formSections.map(({ name, desc, render, required }) => + ${this.formSections.map((section) => panel({ - heading: this.tabLabels[name], - content: html` -

${desc}

-
${render.bind(this)()}
- `, - actions: required + id: `${section.name}--panel`, + className: clsx( + `${formName}--panel`, + section.name === this.progressState?.activeTab && + `${formName}--panel--active`, + ), + heading: this.tabLabels[section.name], + content: content(section), + actions: section.required ? html`

${msg( html`Fields marked with @@ -476,7 +499,7 @@ export class WorkflowEditor extends BtrixElement { size="small" type=${this.configId ? "submit" : "button"} variant=${this.configId ? "primary" : "default"} - ?disabled=${this.isSubmitting || !hasRequiredFields} + ?disabled=${this.isSubmitting} ?loading=${!!this.configId && this.isSubmitting} > ${this.configId ? msg("Save") : msg("Create")} @@ -486,7 +509,6 @@ export class WorkflowEditor extends BtrixElement { type=${this.configId ? "button" : "submit"} variant=${this.configId ? "default" : "primary"} ?disabled=${this.isSubmitting || - !hasRequiredFields || isArchivingDisabled(this.org, true)} ?loading=${!this.configId && this.isSubmitting} > @@ -1483,58 +1505,6 @@ https://archiveweb.page/images/${"logo.svg"}`} `; } - private readonly renderConfirmSettings = () => { - const errorAlert = when(this.formHasError, () => { - const pageScope = isPageScopeType(this.formState.scopeType); - const crawlSetupUrl = `${window.location.href.split("#")[0]}#crawlSetup`; - const errorMessage = this.hasRequiredFields() - ? msg( - "There are issues with this Workflow. Please go through previous steps and fix all issues to continue.", - ) - : html` - ${msg("There is an issue with this Crawl Workflow:")}

- ${msg( - html`${pageScope ? msg("Page URL(s)") : msg("Crawl Start URL")} - required in - Scope. `, - )} -

- ${msg("Please fix to continue.")} - `; - - return this.renderErrorAlert(errorMessage); - }); - - return html` - ${errorAlert} - -

- ${when(this.progressState!.activeTab === "confirmSettings", () => { - // Prevent parsing and rendering tab when not visible - const crawlConfig = this.parseConfig(); - const profileName = this.formState.browserProfile?.name; - - return html` - `; - })} -
- - ${errorAlert} - `; - }; - private readonly formSections: { name: StepName; desc: string; @@ -1615,11 +1585,16 @@ https://archiveweb.page/images/${"logo.svg"}`} private async scrollToPanelTop() { const activeTabPanel = await this.activeTabPanel; - if (activeTabPanel && activeTabPanel.getBoundingClientRect().top < 0) { - activeTabPanel.scrollIntoView({ - behavior: "smooth", - }); + + if (!activeTabPanel) { + console.debug("no activeTabPanel"); + return; } + + // TODO scroll into view if needed + activeTabPanel.scrollIntoView(); + + void (await this.activeTabPanel)?.querySelector("sl-details")?.show(); } private async handleRemoveRegex(e: CustomEvent) { @@ -1675,25 +1650,26 @@ https://archiveweb.page/images/${"logo.svg"}`} }; private syncTabErrorState(el: HTMLElement) { - const panelEl = el.closest("btrix-tab-panel")!; - const tabName = panelEl - .getAttribute("name")! - .replace("newJobConfig-", "") as StepName; - const hasInvalid = panelEl.querySelector("[data-user-invalid]"); - - if (!hasInvalid && this.progressState!.tabs[tabName].error) { - this.updateProgressState({ - tabs: { - [tabName]: { error: false }, - }, - }); - } else if (hasInvalid && !this.progressState!.tabs[tabName].error) { - this.updateProgressState({ - tabs: { - [tabName]: { error: true }, - }, - }); - } + console.log("TODO syncTabErrorState", el); + // const panelEl = el.closest("btrix-tab-panel")!; + // const tabName = panelEl + // .getAttribute("name")! + // .replace("newJobConfig-", "") as StepName; + // const hasInvalid = panelEl.querySelector("[data-user-invalid]"); + + // if (!hasInvalid && this.progressState!.tabs[tabName].error) { + // this.updateProgressState({ + // tabs: { + // [tabName]: { error: false }, + // }, + // }); + // } else if (hasInvalid && !this.progressState!.tabs[tabName].error) { + // this.updateProgressState({ + // tabs: { + // [tabName]: { error: true }, + // }, + // }); + // } } private updateFormStateOnChange(e: Event) { @@ -1738,43 +1714,14 @@ https://archiveweb.page/images/${"logo.svg"}`} e.stopPropagation(); return; } - window.location.hash = step; + this.updateProgressState({ activeTab: step }); }; - private backStep() { - const targetTabIdx = STEPS.indexOf(this.progressState!.activeTab); - if (targetTabIdx) { - this.updateProgressState({ - activeTab: STEPS[targetTabIdx - 1] as StepName, - }); - } - } - - private nextStep() { - const isValid = this.checkCurrentPanelValidity(); - - if (isValid) { - const { activeTab } = this.progressState!; - const nextTab = STEPS[STEPS.indexOf(activeTab) + 1] as StepName; - this.updateProgressState({ - activeTab: nextTab, - tabs: { - [activeTab]: { - completed: true, - }, - }, - }); - } - } - - private readonly checkCurrentPanelValidity = (): boolean => { + private readonly checkCurrentPanelValidity = async (): Promise => { if (!this.formElem) return false; - const currentTab = this.progressState!.activeTab as StepName; - const activePanel = this.formElem.querySelector( - `btrix-tab-panel[name="newJobConfig-${currentTab}"]`, - ); + const activePanel = await this.activeTabPanel; const invalidElems = [...activePanel!.querySelectorAll("[data-invalid]")]; const hasInvalid = Boolean(invalidElems.length); @@ -1815,7 +1762,7 @@ https://archiveweb.page/images/${"logo.svg"}`} private async onSubmit(event: SubmitEvent) { event.preventDefault(); - const isValid = this.checkCurrentPanelValidity(); + const isValid = await this.checkCurrentPanelValidity(); await this.updateComplete; if (!isValid || this.formHasError) { diff --git a/frontend/src/layouts/panel.ts b/frontend/src/layouts/panel.ts index 0d292d5fa8..325062d240 100644 --- a/frontend/src/layouts/panel.ts +++ b/frontend/src/layouts/panel.ts @@ -1,4 +1,5 @@ import { html, type TemplateResult } from "lit"; +import { ifDefined } from "lit/directives/if-defined.js"; import { pageHeading } from "./page"; @@ -30,10 +31,14 @@ export function panel({ content, heading, actions, + id, + className, }: { content: TemplateResult; + id?: string; + className?: string; } & Parameters[0]) { - return html`
+ return html`
${panelHeader({ heading, actions })} ${content}
`; } From d3d0fe92696bf570c6f0b7fe6f9cfb302609ea14 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Tue, 14 Jan 2025 19:22:21 -0800 Subject: [PATCH 05/11] add back indicator --- frontend/src/components/ui/tab-list.ts | 136 ++++-------------- .../crawl-workflows/workflow-editor.ts | 67 +++++---- 2 files changed, 69 insertions(+), 134 deletions(-) diff --git a/frontend/src/components/ui/tab-list.ts b/frontend/src/components/ui/tab-list.ts index 0ee03401d1..8c9c9f558c 100644 --- a/frontend/src/components/ui/tab-list.ts +++ b/frontend/src/components/ui/tab-list.ts @@ -8,50 +8,10 @@ const DEFAULT_PANEL_ID = "default-panel"; // postcss-lit-disable-next-line export const TWO_COL_SCREEN_MIN_CSS = css`64.5rem`; -/** - * @deprecated Use `btrix-tab-group` - * - * Tab list - * - * Usage example: - * ```ts - * - * One - * Two - * - * - * Tab one content - * Tab two content - * ``` - */ - -@customElement("btrix-tab-panel") -export class TabPanel extends TailwindElement { - @property({ type: String }) - name?: string; +const tabTagName = "btrix-tab-list-tab" as const; - @property({ type: Boolean }) - active = false; - - render() { - return html` -
- -
- `; - } -} - -/** - * @deprecated Use `btrix-tab-group` - */ -@customElement("btrix-tab") -export class Tab extends TailwindElement { +@customElement(tabTagName) +export class TabListTab extends TailwindElement { // ID of panel the tab labels/controls @property({ type: String }) name?: string; @@ -65,7 +25,7 @@ export class Tab extends TailwindElement { render() { return html`
`; } - private getPanels(): TabPanelElement[] { - const slotElems = this.renderRoot - .querySelector(".content slot:not([name])")! - .assignedElements(); - return ([...slotElems] as TabPanelElement[]).filter( - (el) => el.tagName.toLowerCase() === "btrix-tab-panel", - ); - } - private getTabs(): TabElement[] { const slotElems = this.renderRoot - .querySelector("slot[name='nav']")! + .querySelector("slot")! .assignedElements(); return ([...slotElems] as TabElement[]).filter( - (el) => el.tagName.toLowerCase() === "btrix-tab", + (el) => el.tagName.toLowerCase() === tabTagName, ); } @@ -305,7 +231,7 @@ export class TabList extends TailwindElement { private onActiveChange(isFirstChange: boolean) { this.getTabs().forEach((tab) => { - if (tab.name === this.activePanel) { + if (tab.name === this.tab) { tab.active = true; if (!this.progressPanel) { @@ -315,15 +241,5 @@ export class TabList extends TailwindElement { tab.active = false; } }); - this.getPanels().forEach((panel) => { - panel.active = panel.name === this.activePanel; - if (panel.active) { - panel.style.display = "flex"; - panel.setAttribute("aria-hidden", "false"); - } else { - panel.style.display = "none"; - panel.setAttribute("aria-hidden", "true"); - } - }); } } diff --git a/frontend/src/features/crawl-workflows/workflow-editor.ts b/frontend/src/features/crawl-workflows/workflow-editor.ts index 6a7251b68c..ca23b83cee 100644 --- a/frontend/src/features/crawl-workflows/workflow-editor.ts +++ b/frontend/src/features/crawl-workflows/workflow-editor.ts @@ -1,7 +1,9 @@ import { consume } from "@lit/context"; import { localized, msg, str } from "@lit/localize"; import type { + SlAfterShowEvent, SlCheckbox, + SlDetails, SlHideEvent, SlInput, SlRadio, @@ -41,7 +43,7 @@ import type { SelectCrawlerUpdateEvent, } from "@/components/ui/select-crawler"; import type { SelectCrawlerProxyChangeEvent } from "@/components/ui/select-crawler-proxy"; -import type { Tab } from "@/components/ui/tab-list"; +import type { TabListTab } from "@/components/ui/tab-list"; import type { TagInputEvent, TagsChangeEvent } from "@/components/ui/tag-input"; import type { TimeInputChangeEvent } from "@/components/ui/time-input"; import { validURL } from "@/components/ui/url-input"; @@ -333,24 +335,13 @@ export class WorkflowEditor extends BtrixElement { this.progressState.activeTab ) { void this.scrollToPanelTop(); - - // Focus on first field in section - (await this.activeTabPanel) - ?.querySelector( - "sl-input, sl-textarea, sl-select, sl-radio-group", - ) - ?.focus(); } } } async firstUpdated() { // Focus on first field in section - (await this.activeTabPanel) - ?.querySelector( - "sl-input, sl-textarea, sl-select, sl-radio-group", - ) - ?.focus(); + this.moveFocus((await this.activeTabPanel)?.querySelector("sl-details")); if (this.orgId) { void this.fetchTags(); @@ -399,9 +390,6 @@ export class WorkflowEditor extends BtrixElement { } private renderNav() { - const baseUrl = `${this.navigate.orgBasePath}/workflows/${ - this.configId ? `${this.configId}?edit` : "new" - }`; let orderedTabNames = STEPS as readonly StepName[]; if (this.configId) { @@ -412,18 +400,21 @@ export class WorkflowEditor extends BtrixElement { const button = (tab: StepName) => { const isActive = tab === this.progressState?.activeTab; return html` - ${this.tabLabels[tab]} - + `; }; - return html` ${orderedTabNames.map(button)} `; + return html` + + ${orderedTabNames.map(button)} + + `; } private renderFormSections() { @@ -438,6 +429,9 @@ export class WorkflowEditor extends BtrixElement { return html` { + this.moveFocus(e.target as SlDetails); + }} @sl-hide=${(e: SlHideEvent) => { if (hasError) { e.preventDefault(); @@ -632,6 +626,7 @@ export class WorkflowEditor extends BtrixElement { autocomplete="off" inputmode="url" value=${this.formState.urlList} + autofocus required @sl-input=${async (e: Event) => { const inputEl = e.target as SlInput; @@ -1594,7 +1589,31 @@ https://archiveweb.page/images/${"logo.svg"}`} // TODO scroll into view if needed activeTabPanel.scrollIntoView(); - void (await this.activeTabPanel)?.querySelector("sl-details")?.show(); + const details = (await this.activeTabPanel)?.querySelector("sl-details"); + + if (details) { + if (details.open) { + this.moveFocus(details); + } else { + void details.show(); + } + } + } + + private moveFocus(details?: SlDetails | null) { + if (!details) return; + + const required = details.querySelector("[required]"); + + if (required) { + required.focus(); + } else { + details + .querySelector( + "sl-input, sl-textarea, sl-select, sl-radio-group", + ) + ?.focus(); + } } private async handleRemoveRegex(e: CustomEvent) { @@ -1708,13 +1727,13 @@ https://archiveweb.page/images/${"logo.svg"}`} } private readonly tabClickHandler = (step: StepName) => (e: MouseEvent) => { - const tab = e.currentTarget as Tab; + const tab = e.currentTarget as TabListTab; if (tab.disabled || tab.active) { e.preventDefault(); e.stopPropagation(); return; } - + window.location.hash = step; this.updateProgressState({ activeTab: step }); }; From 20acbb1a08c5753621958b2a758f79320c0903b8 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Wed, 15 Jan 2025 10:56:49 -0800 Subject: [PATCH 06/11] update indicator on scroll --- frontend/src/components/utils/observable.ts | 21 +-- .../features/archived-items/crawl-queue.ts | 2 +- .../crawl-workflows/workflow-editor.ts | 143 +++++++++++------- frontend/src/layouts/panel.ts | 6 +- 4 files changed, 108 insertions(+), 64 deletions(-) diff --git a/frontend/src/components/utils/observable.ts b/frontend/src/components/utils/observable.ts index 5706e61674..f7b4e26b8f 100644 --- a/frontend/src/components/utils/observable.ts +++ b/frontend/src/components/utils/observable.ts @@ -1,5 +1,5 @@ import { html, LitElement } from "lit"; -import { customElement, query } from "lit/decorators.js"; +import { customElement, property } from "lit/decorators.js"; type IntersectionEventDetail = { entry: IntersectionObserverEntry; @@ -11,23 +11,26 @@ export type IntersectEvent = CustomEvent; * * @example Usage: * ``` - * + * * Observe me! * * ``` * - * @event intersect { entry: IntersectionObserverEntry } + * @fires btrix-intersect { entry: IntersectionObserverEntry } */ @customElement("btrix-observable") export class Observable extends LitElement { - @query(".target") - private readonly target?: HTMLElement; + @property({ type: Object }) + options?: IntersectionObserverInit; private observer?: IntersectionObserver; connectedCallback(): void { super.connectedCallback(); - this.observer = new IntersectionObserver(this.handleIntersect); + this.observer = new IntersectionObserver( + this.handleIntersect, + this.options, + ); } disconnectedCallback(): void { @@ -36,18 +39,18 @@ export class Observable extends LitElement { } firstUpdated() { - this.observer?.observe(this.target!); + this.observer?.observe(this); } private readonly handleIntersect = ([entry]: IntersectionObserverEntry[]) => { this.dispatchEvent( - new CustomEvent("intersect", { + new CustomEvent("btrix-intersect", { detail: { entry }, }), ); }; render() { - return html`
`; + return html``; } } diff --git a/frontend/src/features/archived-items/crawl-queue.ts b/frontend/src/features/archived-items/crawl-queue.ts index 9f6ef825a0..1d07a361b1 100644 --- a/frontend/src/features/archived-items/crawl-queue.ts +++ b/frontend/src/features/archived-items/crawl-queue.ts @@ -199,7 +199,7 @@ export class CrawlQueue extends BtrixElement { ${msg("End of queue")}
`, () => html` - +
& Map, ) { @@ -390,13 +398,6 @@ export class WorkflowEditor extends BtrixElement { } private renderNav() { - let orderedTabNames = STEPS as readonly StepName[]; - - if (this.configId) { - // Remove review tab - orderedTabNames = orderedTabNames.slice(0, -1); - } - const button = (tab: StepName) => { const isActive = tab === this.progressState?.activeTab; return html` @@ -412,13 +413,13 @@ export class WorkflowEditor extends BtrixElement { return html` - ${orderedTabNames.map(button)} + ${STEPS.map(button)} `; } private renderFormSections() { - const content = ({ + const panelBody = ({ name, desc, render, @@ -443,31 +444,48 @@ export class WorkflowEditor extends BtrixElement {
${render.bind(this)()}
`; }; + + const formSection = (section: (typeof this.formSections)[number]) => html` + } + > + ${panel({ + id: `${section.name}--panel`, + className: clsx( + `${formName}--panel`, + section.name === this.progressState?.activeTab && + `${formName}--panel--active`, + ), + heading: this.tabLabels[section.name], + body: panelBody(section), + actions: section.required + ? html`

+ ${msg( + html`Fields marked with + * + are required`, + )} +

` + : undefined, + })} + + `; + return html`
- ${this.formSections.map((section) => - panel({ - id: `${section.name}--panel`, - className: clsx( - `${formName}--panel`, - section.name === this.progressState?.activeTab && - `${formName}--panel--active`, - ), - heading: this.tabLabels[section.name], - content: content(section), - actions: section.required - ? html`

- ${msg( - html`Fields marked with - * - are required`, - )} -

` - : undefined, - }), - )} + ${this.formSections.map(formSection)}
${when(this.serverError, (error) => this.renderErrorAlert(error))} @@ -1570,34 +1588,57 @@ https://archiveweb.page/images/${"logo.svg"}`} this.updateFormState(formState); } - private hasRequiredFields(): boolean { - if (isPageScopeType(this.formState.scopeType)) { - return Boolean(this.formState.urlList); - } + private intersectionEntries: IntersectionObserverEntry[] = []; - return Boolean(this.formState.primarySeedUrl); - } + private readonly onFormSectionIntersect = throttle(8)((e: IntersectEvent) => { + const { entry } = e.detail; - private async scrollToPanelTop() { - const activeTabPanel = await this.activeTabPanel; + const observableEl = entry.target as HTMLElement; + const tab = observableEl.dataset["tab"] as StepName; + const tabIndex = STEPS.indexOf(tab); - if (!activeTabPanel) { - console.debug("no activeTabPanel"); + if (tabIndex === -1) { + console.debug("tab not in steps:", tab); return; } - // TODO scroll into view if needed - activeTabPanel.scrollIntoView(); + this.intersectionEntries[tabIndex] = entry; - const details = (await this.activeTabPanel)?.querySelector("sl-details"); + if (!entry.isIntersecting) return; - if (details) { - if (details.open) { - this.moveFocus(details); - } else { - void details.show(); - } + // TODO Check if this is the only visible tab + const intersecting = this.intersectionEntries.find( + (entry) => entry.isIntersecting, + ); + console.log("intersecting:", tab, intersecting?.target); + + this.updateProgressState({ activeTab: tab as StepName }); + }); + + private hasRequiredFields(): boolean { + if (isPageScopeType(this.formState.scopeType)) { + return Boolean(this.formState.urlList); } + + return Boolean(this.formState.primarySeedUrl); + } + + private async scrollToPanelTop() { + // const activeTabPanel = await this.activeTabPanel; + // if (!activeTabPanel) { + // console.debug("no activeTabPanel"); + // return; + // } + // // TODO scroll into view if needed + // activeTabPanel.scrollIntoView(); + // const details = (await this.activeTabPanel)?.querySelector("sl-details"); + // if (details) { + // if (details.open) { + // this.moveFocus(details); + // } else { + // // void details.show(); + // } + // } } private moveFocus(details?: SlDetails | null) { diff --git a/frontend/src/layouts/panel.ts b/frontend/src/layouts/panel.ts index 325062d240..6bfe56fb2a 100644 --- a/frontend/src/layouts/panel.ts +++ b/frontend/src/layouts/panel.ts @@ -28,17 +28,17 @@ export function panelBody({ content }: { content: TemplateResult }) { * @TODO Refactor components to use panel */ export function panel({ - content, heading, actions, + body, id, className, }: { - content: TemplateResult; + body: TemplateResult; id?: string; className?: string; } & Parameters[0]) { return html`
- ${panelHeader({ heading, actions })} ${content} + ${panelHeader({ heading, actions })} ${body}
`; } From 1eafb0472e1ababe39c05b783928d7b6bd2d49e3 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Wed, 22 Jan 2025 16:30:04 -0800 Subject: [PATCH 07/11] refactor observable --- frontend/src/components/utils/observable.ts | 32 +----- frontend/src/controllers/observable.ts | 50 ++++++++ .../features/archived-items/crawl-queue.ts | 5 +- .../crawl-workflows/workflow-editor.ts | 107 +++++++++--------- 4 files changed, 112 insertions(+), 82 deletions(-) create mode 100644 frontend/src/controllers/observable.ts diff --git a/frontend/src/components/utils/observable.ts b/frontend/src/components/utils/observable.ts index f7b4e26b8f..6f73401ab9 100644 --- a/frontend/src/components/utils/observable.ts +++ b/frontend/src/components/utils/observable.ts @@ -1,10 +1,7 @@ import { html, LitElement } from "lit"; import { customElement, property } from "lit/decorators.js"; -type IntersectionEventDetail = { - entry: IntersectionObserverEntry; -}; -export type IntersectEvent = CustomEvent; +import { ObservableController } from "@/controllers/observable"; /** * Observe element with Intersection Observer API. @@ -16,40 +13,19 @@ export type IntersectEvent = CustomEvent; * * ``` * - * @fires btrix-intersect { entry: IntersectionObserverEntry } + * @fires btrix-intersect IntersectionEventDetail */ @customElement("btrix-observable") export class Observable extends LitElement { @property({ type: Object }) options?: IntersectionObserverInit; - private observer?: IntersectionObserver; - - connectedCallback(): void { - super.connectedCallback(); - this.observer = new IntersectionObserver( - this.handleIntersect, - this.options, - ); - } - - disconnectedCallback(): void { - this.observer?.disconnect(); - super.disconnectedCallback(); - } + private readonly observable = new ObservableController(this); firstUpdated() { - this.observer?.observe(this); + this.observable.observe(this); } - private readonly handleIntersect = ([entry]: IntersectionObserverEntry[]) => { - this.dispatchEvent( - new CustomEvent("btrix-intersect", { - detail: { entry }, - }), - ); - }; - render() { return html``; } diff --git a/frontend/src/controllers/observable.ts b/frontend/src/controllers/observable.ts new file mode 100644 index 0000000000..8234e72c7c --- /dev/null +++ b/frontend/src/controllers/observable.ts @@ -0,0 +1,50 @@ +import type { ReactiveController, ReactiveControllerHost } from "lit"; + +type IntersectionEventDetail = { + entries: IntersectionObserverEntry[]; +}; +export type IntersectEvent = CustomEvent; + +/** + * Observe one or more elements with Intersection Observer API. + * + * @fires btrix-intersect IntersectionEventDetail + */ +export class ObservableController implements ReactiveController { + private readonly host: ReactiveControllerHost & EventTarget; + + private observer?: IntersectionObserver; + private readonly observerOptions?: IntersectionObserverInit; + + constructor( + host: ObservableController["host"], + options?: IntersectionObserverInit, + ) { + this.host = host; + this.observerOptions = options; + host.addController(this); + } + + hostConnected() { + this.observer = new IntersectionObserver( + this.handleIntersect, + this.observerOptions, + ); + } + + hostDisconnected() { + this.observer?.disconnect(); + } + + public observe(target: Element) { + this.observer?.observe(target); + } + + private readonly handleIntersect = (entries: IntersectionObserverEntry[]) => { + this.host.dispatchEvent( + new CustomEvent("btrix-intersect", { + detail: { entries }, + }), + ); + }; +} diff --git a/frontend/src/features/archived-items/crawl-queue.ts b/frontend/src/features/archived-items/crawl-queue.ts index 1d07a361b1..f194f09e8c 100644 --- a/frontend/src/features/archived-items/crawl-queue.ts +++ b/frontend/src/features/archived-items/crawl-queue.ts @@ -10,6 +10,7 @@ import { when } from "lit/directives/when.js"; import throttle from "lodash/fp/throttle"; import { BtrixElement } from "@/classes/BtrixElement"; +import type { IntersectEvent } from "@/controllers/observable"; type Pages = string[]; type ResponseData = { @@ -230,8 +231,8 @@ export class CrawlQueue extends BtrixElement { `; } - private readonly onLoadMoreIntersect = throttle(50)((e: CustomEvent) => { - if (!e.detail.entry.isIntersecting) return; + private readonly onLoadMoreIntersect = throttle(50)((e: IntersectEvent) => { + if (!e.detail.entries[0].isIntersecting) return; this.loadMore(); }) as (e: CustomEvent) => void; diff --git a/frontend/src/features/crawl-workflows/workflow-editor.ts b/frontend/src/features/crawl-workflows/workflow-editor.ts index 64a1a0ffe6..10b9757fe2 100644 --- a/frontend/src/features/crawl-workflows/workflow-editor.ts +++ b/frontend/src/features/crawl-workflows/workflow-editor.ts @@ -26,6 +26,7 @@ import { customElement, property, query, + queryAll, queryAsync, state, } from "lit/decorators.js"; @@ -48,8 +49,11 @@ import type { TabListTab } from "@/components/ui/tab-list"; import type { TagInputEvent, TagsChangeEvent } from "@/components/ui/tag-input"; import type { TimeInputChangeEvent } from "@/components/ui/time-input"; import { validURL } from "@/components/ui/url-input"; -import type { IntersectEvent } from "@/components/utils/observable"; import { proxiesContext, type ProxiesContext } from "@/context/org"; +import { + ObservableController, + type IntersectEvent, +} from "@/controllers/observable"; import { type SelectBrowserProfileChangeEvent } from "@/features/browser-profiles/select-browser-profile"; import type { CollectionsChangeEvent } from "@/features/collections/collections-add"; import type { QueueExclusionTable } from "@/features/crawl-workflows/queue-exclusion-table"; @@ -117,6 +121,7 @@ const DEFAULT_BEHAVIORS = [ "siteSpecific", ]; const formName = "newJobConfig" as const; +const panelSuffix = "--panel" as const; const getDefaultProgressState = (hasConfigId = false): ProgressState => { let activeTab: StepName = "crawlSetup"; @@ -220,6 +225,12 @@ export class WorkflowEditor extends BtrixElement { @state() private serverError?: TemplateResult | string; + // For observing panel sections position in viewport + private readonly observable = new ObservableController(this, { + // Add some padding to account for stickied elements + rootMargin: "-100px 0px -100px 0px", + }); + // For fuzzy search: private readonly fuse = new Fuse([], { shouldSort: false, @@ -280,7 +291,10 @@ export class WorkflowEditor extends BtrixElement { @query(`form[name="${formName}"]`) private readonly formElem?: HTMLFormElement; - @queryAsync(`.${formName}--panel--active`) + @queryAll(`.${formName}${panelSuffix}`) + private readonly panels?: HTMLElement[]; + + @queryAsync(`.${formName}${panelSuffix}--active`) private readonly activeTabPanel!: Promise; connectedCallback(): void { @@ -288,6 +302,13 @@ export class WorkflowEditor extends BtrixElement { super.connectedCallback(); void this.fetchServerDefaults(); + this.addEventListener( + "btrix-intersect", + this.onFormSectionIntersect as UnderlyingFunction< + typeof this.onFormSectionIntersect + >, + ); + window.addEventListener("hashchange", () => { const hashValue = window.location.hash.slice(1); if (STEPS.includes(hashValue as (typeof STEPS)[number])) { @@ -348,6 +369,11 @@ export class WorkflowEditor extends BtrixElement { } async firstUpdated() { + // Observe form sections to get scroll position + this.panels?.forEach((panel) => { + this.observable.observe(panel); + }); + // Focus on first field in section this.moveFocus((await this.activeTabPanel)?.querySelector("sl-details")); @@ -446,41 +472,27 @@ export class WorkflowEditor extends BtrixElement { }; const formSection = (section: (typeof this.formSections)[number]) => html` - } - > - ${panel({ - id: `${section.name}--panel`, - className: clsx( - `${formName}--panel`, - section.name === this.progressState?.activeTab && - `${formName}--panel--active`, - ), - heading: this.tabLabels[section.name], - body: panelBody(section), - actions: section.required - ? html`

- ${msg( - html`Fields marked with - * - are required`, - )} -

` - : undefined, - })} - + ${panel({ + id: `${section.name}${panelSuffix}`, + className: clsx( + `${formName}${panelSuffix}`, + section.name === this.progressState?.activeTab && + `${formName}${panelSuffix}--active`, + ), + heading: this.tabLabels[section.name], + body: panelBody(section), + actions: section.required + ? html`

+ ${msg( + html`Fields marked with + * + are required`, + )} +

` + : undefined, + })} `; return html` @@ -1588,13 +1600,14 @@ https://archiveweb.page/images/${"logo.svg"}`} this.updateFormState(formState); } - private intersectionEntries: IntersectionObserverEntry[] = []; + private readonly onFormSectionIntersect = throttle(10)((e: Event) => { + const { entries } = (e as IntersectEvent).detail; + const entry = entries.find((entry) => entry.isIntersecting); - private readonly onFormSectionIntersect = throttle(8)((e: IntersectEvent) => { - const { entry } = e.detail; + if (!entry?.isIntersecting) return; + + const tab = entry.target.id.split(panelSuffix)[0] as StepName; - const observableEl = entry.target as HTMLElement; - const tab = observableEl.dataset["tab"] as StepName; const tabIndex = STEPS.indexOf(tab); if (tabIndex === -1) { @@ -1602,16 +1615,6 @@ https://archiveweb.page/images/${"logo.svg"}`} return; } - this.intersectionEntries[tabIndex] = entry; - - if (!entry.isIntersecting) return; - - // TODO Check if this is the only visible tab - const intersecting = this.intersectionEntries.find( - (entry) => entry.isIntersecting, - ); - console.log("intersecting:", tab, intersecting?.target); - this.updateProgressState({ activeTab: tab as StepName }); }); From 93a1c8dccc2049569d158fdc144de6b07a6b038e Mon Sep 17 00:00:00 2001 From: sua yoo Date: Wed, 22 Jan 2025 18:42:22 -0800 Subject: [PATCH 08/11] handle scroll --- .../crawl-workflows/workflow-editor.ts | 136 ++++++++++-------- 1 file changed, 80 insertions(+), 56 deletions(-) diff --git a/frontend/src/features/crawl-workflows/workflow-editor.ts b/frontend/src/features/crawl-workflows/workflow-editor.ts index 10b9757fe2..7dec7673c6 100644 --- a/frontend/src/features/crawl-workflows/workflow-editor.ts +++ b/frontend/src/features/crawl-workflows/workflow-editor.ts @@ -231,6 +231,10 @@ export class WorkflowEditor extends BtrixElement { rootMargin: "-100px 0px -100px 0px", }); + // Use to skip updates on intersect changes, like when scrolling + // an element into view on click + private skipIntersectUpdate = false; + // For fuzzy search: private readonly fuse = new Fuse([], { shouldSort: false, @@ -308,15 +312,6 @@ export class WorkflowEditor extends BtrixElement { typeof this.onFormSectionIntersect >, ); - - window.addEventListener("hashchange", () => { - const hashValue = window.location.hash.slice(1); - if (STEPS.includes(hashValue as (typeof STEPS)[number])) { - this.updateProgressState({ - activeTab: hashValue as StepName, - }); - } - }); } disconnectedCallback(): void { @@ -355,16 +350,15 @@ export class WorkflowEditor extends BtrixElement { } } - async updated( - changedProperties: PropertyValues & Map, - ) { - if (changedProperties.get("progressState") && this.progressState) { - if ( - (changedProperties.get("progressState") as ProgressState).activeTab !== - this.progressState.activeTab - ) { - void this.scrollToPanelTop(); - } + updated(changedProperties: PropertyValues & Map) { + if ( + changedProperties.has("progressState") && + this.progressState && + this.progressState.activeTab !== + (changedProperties.get("progressState") as ProgressState | undefined) + ?.activeTab + ) { + window.location.hash = this.progressState.activeTab; } } @@ -374,8 +368,7 @@ export class WorkflowEditor extends BtrixElement { this.observable.observe(panel); }); - // Focus on first field in section - this.moveFocus((await this.activeTabPanel)?.querySelector("sl-details")); + void this.scrollToActivePanel(); if (this.orgId) { void this.fetchTags(); @@ -456,14 +449,26 @@ export class WorkflowEditor extends BtrixElement { return html` { + this.skipIntersectUpdate = true; + this.updateProgressState({ + activeTab: name, + }); + }} @sl-after-show=${(e: SlAfterShowEvent) => { this.moveFocus(e.target as SlDetails); + this.skipIntersectUpdate = false; }} @sl-hide=${(e: SlHideEvent) => { if (hasError) { e.preventDefault(); // TODO focus on error } + + this.skipIntersectUpdate = true; + }} + @sl-after-hide=${() => { + this.skipIntersectUpdate = false; }} >

${desc}

@@ -477,7 +482,7 @@ export class WorkflowEditor extends BtrixElement { className: clsx( `${formName}${panelSuffix}`, section.name === this.progressState?.activeTab && - `${formName}${panelSuffix}--active`, + `${formName}${panelSuffix}--active scroll-mt-7`, ), heading: this.tabLabels[section.name], body: panelBody(section), @@ -496,7 +501,7 @@ export class WorkflowEditor extends BtrixElement { `; return html` -
+
${this.formSections.map(formSection)}
@@ -1604,18 +1609,23 @@ https://archiveweb.page/images/${"logo.svg"}`} const { entries } = (e as IntersectEvent).detail; const entry = entries.find((entry) => entry.isIntersecting); - if (!entry?.isIntersecting) return; + if (!entry) return; const tab = entry.target.id.split(panelSuffix)[0] as StepName; - const tabIndex = STEPS.indexOf(tab); + if (this.skipIntersectUpdate) { + this.onFormSectionIntersect.cancel(); + this.skipIntersectUpdate = false; + } else { + const tabIndex = STEPS.indexOf(tab); - if (tabIndex === -1) { - console.debug("tab not in steps:", tab); - return; - } + if (tabIndex === -1) { + console.debug("tab not in steps:", tab); + return; + } - this.updateProgressState({ activeTab: tab as StepName }); + this.updateProgressState({ activeTab: tab as StepName }); + } }); private hasRequiredFields(): boolean { @@ -1626,22 +1636,31 @@ https://archiveweb.page/images/${"logo.svg"}`} return Boolean(this.formState.primarySeedUrl); } - private async scrollToPanelTop() { - // const activeTabPanel = await this.activeTabPanel; - // if (!activeTabPanel) { - // console.debug("no activeTabPanel"); - // return; - // } - // // TODO scroll into view if needed - // activeTabPanel.scrollIntoView(); - // const details = (await this.activeTabPanel)?.querySelector("sl-details"); - // if (details) { - // if (details.open) { - // this.moveFocus(details); - // } else { - // // void details.show(); - // } - // } + private async scrollToActivePanel() { + const activeTabPanel = await this.activeTabPanel; + if (!activeTabPanel) { + console.debug("no activeTabPanel"); + return; + } + + this.onFormSectionIntersect.cancel(); + this.skipIntersectUpdate = true; + + const details = activeTabPanel.querySelector("sl-details"); + + if (!details) { + console.debug("no sl-details in:", this.activeTabPanel); + + activeTabPanel.scrollIntoView(); + return; + } + + if (details.open) { + // Moving focus will scroll that section into view + this.moveFocus(details); + } else { + activeTabPanel.scrollIntoView(); + } } private moveFocus(details?: SlDetails | null) { @@ -1770,16 +1789,21 @@ https://archiveweb.page/images/${"logo.svg"}`} }); } - private readonly tabClickHandler = (step: StepName) => (e: MouseEvent) => { - const tab = e.currentTarget as TabListTab; - if (tab.disabled || tab.active) { - e.preventDefault(); - e.stopPropagation(); - return; - } - window.location.hash = step; - this.updateProgressState({ activeTab: step }); - }; + private readonly tabClickHandler = + (step: StepName) => async (e: MouseEvent) => { + const tab = e.currentTarget as TabListTab; + if (tab.disabled || tab.active) { + e.preventDefault(); + e.stopPropagation(); + return; + } + + this.updateProgressState({ activeTab: step }); + + await this.updateComplete; + + void this.scrollToActivePanel(); + }; private readonly checkCurrentPanelValidity = async (): Promise => { if (!this.formElem) return false; From f542e60551994c0b25980cbc31dcf5be291c41f9 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Wed, 22 Jan 2025 20:34:11 -0800 Subject: [PATCH 09/11] handle form errors --- .../crawl-workflows/workflow-editor.ts | 224 ++++++++++++------ 1 file changed, 148 insertions(+), 76 deletions(-) diff --git a/frontend/src/features/crawl-workflows/workflow-editor.ts b/frontend/src/features/crawl-workflows/workflow-editor.ts index 7dec7673c6..583bf23ac7 100644 --- a/frontend/src/features/crawl-workflows/workflow-editor.ts +++ b/frontend/src/features/crawl-workflows/workflow-editor.ts @@ -73,7 +73,7 @@ import { humanizeNextDate, humanizeSchedule, } from "@/utils/cron"; -import { maxLengthValidator } from "@/utils/form"; +import { formValidator, maxLengthValidator } from "@/utils/form"; import localize from "@/utils/localize"; import { isArchivingDisabled } from "@/utils/orgs"; import { AppStateService } from "@/utils/state"; @@ -241,6 +241,7 @@ export class WorkflowEditor extends BtrixElement { threshold: 0.2, // stricter; default is 0.6 }); + private readonly checkFormValidity = formValidator(this); private readonly validateNameMax = maxLengthValidator(50); private readonly validateDescriptionMax = maxLengthValidator(350); @@ -447,7 +448,14 @@ export class WorkflowEditor extends BtrixElement { const hasError = this.progressState?.tabs[name].error; return html` { this.skipIntersectUpdate = true; @@ -460,9 +468,16 @@ export class WorkflowEditor extends BtrixElement { this.skipIntersectUpdate = false; }} @sl-hide=${(e: SlHideEvent) => { - if (hasError) { + // Check if there's any invalid elements before hiding + const invalidEl = ( + e.currentTarget as SlDetails + ).querySelector("[data-user-invalid]"); + + if (invalidEl) { e.preventDefault(); - // TODO focus on error + + invalidEl.focus(); + invalidEl.checkValidity(); } this.skipIntersectUpdate = true; @@ -471,7 +486,36 @@ export class WorkflowEditor extends BtrixElement { this.skipIntersectUpdate = false; }} > -

${desc}

+
+ + + +
+
+ ${when( + hasError, + () => html` + + + + `, + () => html` + + `, + )} +
+ +

${desc}

${render.bind(this)()}
`; }; @@ -510,40 +554,65 @@ export class WorkflowEditor extends BtrixElement { } private renderFooter() { - const hasRequiredFields = this.hasRequiredFields(); + let primaryButton = html` + + ${msg("Save & Run Workflow")} + + `; + let secondaryButton = html` + + ${msg("Only Save")} + + `; + + if (this.configId) { + primaryButton = html` + + ${msg("Save Workflow")} + + `; + secondaryButton = html` + + ${msg("Save & Run")} + + `; + } return html`
- - ${msg("Cancel")} - - - - ${this.configId ? msg("Save") : msg("Create")} - - - ${this.configId ? msg("Save and Run") : msg("Run Workflow")} - - + ${this.configId + ? html` + + ${msg("Cancel")} + + ` + : nothing} + ${primaryButton}${secondaryButton}
`; } @@ -1717,7 +1786,14 @@ https://archiveweb.page/images/${"logo.svg"}`} await el.updateComplete; await this.updateComplete; - const currentTab = this.progressState!.activeTab as StepName; + const panelEl = el.closest(`.${formName}${panelSuffix}`); + + if (!panelEl) { + console.debug("no panel for element:", el); + return; + } + + const currentTab = panelEl.id.split(panelSuffix)[0] as StepName; // Check [data-user-invalid] to validate only touched inputs if ("userInvalid" in el.dataset) { if (this.progressState!.tabs[currentTab].error) return; @@ -1732,26 +1808,29 @@ https://archiveweb.page/images/${"logo.svg"}`} }; private syncTabErrorState(el: HTMLElement) { - console.log("TODO syncTabErrorState", el); - // const panelEl = el.closest("btrix-tab-panel")!; - // const tabName = panelEl - // .getAttribute("name")! - // .replace("newJobConfig-", "") as StepName; - // const hasInvalid = panelEl.querySelector("[data-user-invalid]"); - - // if (!hasInvalid && this.progressState!.tabs[tabName].error) { - // this.updateProgressState({ - // tabs: { - // [tabName]: { error: false }, - // }, - // }); - // } else if (hasInvalid && !this.progressState!.tabs[tabName].error) { - // this.updateProgressState({ - // tabs: { - // [tabName]: { error: true }, - // }, - // }); - // } + const panelEl = el.closest(`.${formName}${panelSuffix}`); + + if (!panelEl) { + console.debug("no panel for element:", el); + return; + } + + const tabName = panelEl.id.split(panelSuffix)[0] as StepName; + const hasInvalid = panelEl.querySelector("[data-user-invalid]"); + + if (!hasInvalid && this.progressState!.tabs[tabName].error) { + this.updateProgressState({ + tabs: { + [tabName]: { error: false }, + }, + }); + } else if (hasInvalid && !this.progressState!.tabs[tabName].error) { + this.updateProgressState({ + tabs: { + [tabName]: { error: true }, + }, + }); + } } private updateFormStateOnChange(e: Event) { @@ -1805,22 +1884,6 @@ https://archiveweb.page/images/${"logo.svg"}`} void this.scrollToActivePanel(); }; - private readonly checkCurrentPanelValidity = async (): Promise => { - if (!this.formElem) return false; - - const activePanel = await this.activeTabPanel; - const invalidElems = [...activePanel!.querySelectorAll("[data-invalid]")]; - - const hasInvalid = Boolean(invalidElems.length); - if (hasInvalid) { - invalidElems.forEach((el) => { - (el as HTMLInputElement).reportValidity(); - }); - } - - return !hasInvalid; - }; - private onKeyDown(event: KeyboardEvent) { const el = event.target as HTMLElement; const tagName = el.tagName.toLowerCase(); @@ -1847,10 +1910,16 @@ https://archiveweb.page/images/${"logo.svg"}`} } } - private async onSubmit(event: SubmitEvent) { + private onSubmit(event: SubmitEvent) { event.preventDefault(); - const isValid = await this.checkCurrentPanelValidity(); - await this.updateComplete; + + void this.save(); + } + + private async save() { + if (!this.formElem) return; + + const isValid = await this.checkFormValidity(this.formElem); if (!isValid || this.formHasError) { return; @@ -1932,7 +2001,10 @@ https://archiveweb.page/images/${"logo.svg"}`} } private async onReset() { - this.initializeEditor(); + this.navigate.to( + `${this.navigate.orgBasePath}/workflows${this.configId ? `/${this.configId}#settings` : ""}`, + ); + // this.initializeEditor(); } /** From 72d80fe52067dadef7eb507e9c9dc9c7b5bb94ed Mon Sep 17 00:00:00 2001 From: sua yoo Date: Wed, 22 Jan 2025 21:50:34 -0800 Subject: [PATCH 10/11] prevent default run now --- .../crawl-workflows/workflow-editor.ts | 148 +++++------------- frontend/src/utils/workflow.ts | 3 - 2 files changed, 39 insertions(+), 112 deletions(-) diff --git a/frontend/src/features/crawl-workflows/workflow-editor.ts b/frontend/src/features/crawl-workflows/workflow-editor.ts index 583bf23ac7..081cc1f5e0 100644 --- a/frontend/src/features/crawl-workflows/workflow-editor.ts +++ b/frontend/src/features/crawl-workflows/workflow-editor.ts @@ -1,7 +1,6 @@ import { consume } from "@lit/context"; import { localized, msg, str } from "@lit/localize"; import type { - SlAfterShowEvent, SlCheckbox, SlDetails, SlHideEvent, @@ -122,6 +121,8 @@ const DEFAULT_BEHAVIORS = [ ]; const formName = "newJobConfig" as const; const panelSuffix = "--panel" as const; +const saveRunButtonName = "saveRun--button" as const; +const saveButtonName = "save--button" as const; const getDefaultProgressState = (hasConfigId = false): ProgressState => { let activeTab: StepName = "crawlSetup"; @@ -135,6 +136,7 @@ const getDefaultProgressState = (hasConfigId = false): ProgressState => { return { activeTab, + // TODO Mark as completed only if form section has data tabs: { crawlSetup: { error: false, completed: hasConfigId }, crawlLimits: { @@ -330,25 +332,6 @@ export class WorkflowEditor extends BtrixElement { this.initializeEditor(); } } - if (changedProperties.get("progressState") && this.progressState) { - if ( - (changedProperties.get("progressState") as ProgressState).activeTab === - "crawlSetup" && - this.progressState.activeTab !== "crawlSetup" - ) { - // Show that required tab has error even if input hasn't been touched - if ( - !this.hasRequiredFields() && - !this.progressState.tabs.crawlSetup.error - ) { - this.updateProgressState({ - tabs: { - crawlSetup: { error: true }, - }, - }); - } - } - } } updated(changedProperties: PropertyValues & Map) { @@ -369,7 +352,9 @@ export class WorkflowEditor extends BtrixElement { this.observable.observe(panel); }); - void this.scrollToActivePanel(); + if (this.progressState?.activeTab !== STEPS[0]) { + void this.scrollToActivePanel(); + } if (this.orgId) { void this.fetchTags(); @@ -445,7 +430,8 @@ export class WorkflowEditor extends BtrixElement { render, required, }: (typeof this.formSections)[number]) => { - const hasError = this.progressState?.tabs[name].error; + const tabProgress = this.progressState?.tabs[name]; + const hasError = tabProgress?.error; return html` { this.skipIntersectUpdate = true; this.updateProgressState({ activeTab: name, }); }} - @sl-after-show=${(e: SlAfterShowEvent) => { - this.moveFocus(e.target as SlDetails); - this.skipIntersectUpdate = false; - }} @sl-hide=${(e: SlHideEvent) => { // Check if there's any invalid elements before hiding const invalidEl = ( @@ -482,6 +464,9 @@ export class WorkflowEditor extends BtrixElement { this.skipIntersectUpdate = true; }} + @sl-after-show=${() => { + this.skipIntersectUpdate = false; + }} @sl-after-hide=${() => { this.skipIntersectUpdate = false; }} @@ -554,50 +539,6 @@ export class WorkflowEditor extends BtrixElement { } private renderFooter() { - let primaryButton = html` - - ${msg("Save & Run Workflow")} - - `; - let secondaryButton = html` - - ${msg("Only Save")} - - `; - - if (this.configId) { - primaryButton = html` - - ${msg("Save Workflow")} - - `; - secondaryButton = html` - - ${msg("Save & Run")} - - `; - } - return html`
` : nothing} - ${primaryButton}${secondaryButton} + + ${msg(html`Save & Run Crawl`)} + + + ${msg("Save")} +
`; } @@ -1714,38 +1673,7 @@ https://archiveweb.page/images/${"logo.svg"}`} this.onFormSectionIntersect.cancel(); this.skipIntersectUpdate = true; - - const details = activeTabPanel.querySelector("sl-details"); - - if (!details) { - console.debug("no sl-details in:", this.activeTabPanel); - - activeTabPanel.scrollIntoView(); - return; - } - - if (details.open) { - // Moving focus will scroll that section into view - this.moveFocus(details); - } else { - activeTabPanel.scrollIntoView(); - } - } - - private moveFocus(details?: SlDetails | null) { - if (!details) return; - - const required = details.querySelector("[required]"); - - if (required) { - required.focus(); - } else { - details - .querySelector( - "sl-input, sl-textarea, sl-select, sl-radio-group", - ) - ?.focus(); - } + activeTabPanel.scrollIntoView(); } private async handleRemoveRegex(e: CustomEvent) { @@ -1910,15 +1838,17 @@ https://archiveweb.page/images/${"logo.svg"}`} } } - private onSubmit(event: SubmitEvent) { + private async onSubmit(event: SubmitEvent) { event.preventDefault(); - void this.save(); - } - - private async save() { if (!this.formElem) return; + if (event.submitter?.getAttribute("name") === saveRunButtonName) { + this.updateFormState({ + runNow: true, + }); + } + const isValid = await this.checkFormValidity(this.formElem); if (!isValid || this.formHasError) { diff --git a/frontend/src/utils/workflow.ts b/frontend/src/utils/workflow.ts index ad1010c24e..85d1016461 100644 --- a/frontend/src/utils/workflow.ts +++ b/frontend/src/utils/workflow.ts @@ -155,9 +155,6 @@ export function getInitialFormState(params: { org?: OrgData | null; }): FormState { const defaultFormState = getDefaultFormState(); - if (!params.configId) { - defaultFormState.runNow = true; - } if (!params.initialWorkflow) return defaultFormState; const formState: Partial = {}; const seedsConfig = params.initialWorkflow.config; From 660639856690c5ba2c25e17755f43b5ac4d918d8 Mon Sep 17 00:00:00 2001 From: sua yoo Date: Wed, 22 Jan 2025 22:24:28 -0800 Subject: [PATCH 11/11] clean up --- .../crawl-workflows/workflow-editor.ts | 61 ++++++++++++------- 1 file changed, 38 insertions(+), 23 deletions(-) diff --git a/frontend/src/features/crawl-workflows/workflow-editor.ts b/frontend/src/features/crawl-workflows/workflow-editor.ts index 081cc1f5e0..bf19f3e3c4 100644 --- a/frontend/src/features/crawl-workflows/workflow-editor.ts +++ b/frontend/src/features/crawl-workflows/workflow-editor.ts @@ -121,8 +121,6 @@ const DEFAULT_BEHAVIORS = [ ]; const formName = "newJobConfig" as const; const panelSuffix = "--panel" as const; -const saveRunButtonName = "saveRun--button" as const; -const saveButtonName = "save--button" as const; const getDefaultProgressState = (hasConfigId = false): ProgressState => { let activeTab: StepName = "crawlSetup"; @@ -450,10 +448,17 @@ export class WorkflowEditor extends BtrixElement { }); }} @sl-hide=${(e: SlHideEvent) => { + const el = e.currentTarget as SlDetails; + // Check if there's any invalid elements before hiding - const invalidEl = ( - e.currentTarget as SlDetails - ).querySelector("[data-user-invalid]"); + let invalidEl: SlInput | null = null; + + if (required) { + invalidEl = el.querySelector("[required][data-invalid]"); + } + + invalidEl = + invalidEl || el.querySelector("[data-user-invalid]"); if (invalidEl) { e.preventDefault(); @@ -490,13 +495,16 @@ export class WorkflowEditor extends BtrixElement { > `, - () => html` - - `, + () => + !required || this.configId + ? html` + + ` + : nothing, )}
@@ -554,7 +562,6 @@ export class WorkflowEditor extends BtrixElement { ` : nothing} ${msg("Save")} @@ -602,6 +609,7 @@ export class WorkflowEditor extends BtrixElement { name="scopeType" label=${msg("Crawl Scope")} value=${this.formState.scopeType} + hoist @sl-change=${(e: Event) => this.changeScopeType( (e.target as HTMLSelectElement).value as FormState["scopeType"], @@ -1717,7 +1725,6 @@ https://archiveweb.page/images/${"logo.svg"}`} const panelEl = el.closest(`.${formName}${panelSuffix}`); if (!panelEl) { - console.debug("no panel for element:", el); return; } @@ -1841,17 +1848,20 @@ https://archiveweb.page/images/${"logo.svg"}`} private async onSubmit(event: SubmitEvent) { event.preventDefault(); - if (!this.formElem) return; + this.updateFormState({ + runNow: true, + }); - if (event.submitter?.getAttribute("name") === saveRunButtonName) { - this.updateFormState({ - runNow: true, - }); - } + void this.save(); + } + + private async save() { + if (!this.formElem) return; const isValid = await this.checkFormValidity(this.formElem); if (!isValid || this.formHasError) { + this.formElem.reportValidity(); return; } @@ -1923,7 +1933,12 @@ https://archiveweb.page/images/${"logo.svg"}`} } } } else { - this.serverError = msg("Something unexpected went wrong"); + this.notify.toast({ + message: msg("Sorry, couldn't save workflow at this time."), + variant: "danger", + icon: "exclamation-octagon", + id: "workflow-created-status", + }); } }