diff --git a/.github/workflows/storybook.yml b/.github/workflows/storybook.yml index 1d6060ab862..97a8302308d 100644 --- a/.github/workflows/storybook.yml +++ b/.github/workflows/storybook.yml @@ -32,12 +32,18 @@ jobs: # output. The deploy jobs only run once this succeeds, so we never publish a # Storybook built from assets that don't compile. Results are cached and # restored by the parallel deploy jobs below. + # + # The cache is saved under a key unique to this run, and restored by the + # content-hash prefix. A fixed key let a run cancelled mid-save (by + # cancel-in-progress) hold the reservation, so the next run couldn't save, + # and its deploy jobs restored nothing and failed to find Storybook. build-assets: name: 'Build assets' runs-on: ubuntu-latest timeout-minutes: 15 outputs: cache-key: ${{ steps.cache-key.outputs.value }} + cache-prefix: ${{ steps.cache-key.outputs.prefix }} deploy-branch: ${{ steps.branch.outputs.value }} steps: - name: Checkout code @@ -60,11 +66,15 @@ jobs: # Keep this separate from frontend CI so deployment jobs do not race # another workflow saving the same cache. run: | - echo "value=storybook-v1-${{ runner.os }}-${{ hashFiles('pnpm-lock.yaml', 'pnpm-workspace.yaml', 'composer.lock', 'package.json', 'packages/*/package.json', 'packages/craftcms-ui/src/**', 'packages/craftcms-ui/tsconfig*.json', 'packages/craftcms-ui/vite.config.*', 'packages/craftcms-ui/tsdown.config.*', 'packages/craftcms-garnish/src/**', 'packages/craftcms-garnish/tsconfig*.json', 'packages/craftcms-garnish/vite.config.*', 'routes/**/*.php', 'src/**/*.php', 'workbench/**/*.php') }}" >> "$GITHUB_OUTPUT" + prefix="storybook-v1-${{ runner.os }}-${{ hashFiles('pnpm-lock.yaml', 'pnpm-workspace.yaml', 'composer.lock', 'package.json', 'packages/*/package.json', 'packages/craftcms-ui/src/**', 'packages/craftcms-ui/tsconfig*.json', 'packages/craftcms-ui/vite.config.*', 'packages/craftcms-ui/tsdown.config.*', 'packages/craftcms-garnish/src/**', 'packages/craftcms-garnish/tsconfig*.json', 'packages/craftcms-garnish/vite.config.*', 'routes/**/*.php', 'src/**/*.php', 'workbench/**/*.php') }}-" + echo "prefix=$prefix" >> "$GITHUB_OUTPUT" + echo "value=${prefix}${{ github.run_id }}-${{ github.run_attempt }}" >> "$GITHUB_OUTPUT" + # Any earlier run with the same inputs will do; a match skips the + # install and build steps, and the save at the end. - name: Restore install + build + generated cache id: cache - uses: actions/cache@v4 + uses: actions/cache/restore@v4 with: path: | node_modules @@ -78,14 +88,15 @@ jobs: resources/js/wayfinder resources/js/generated key: ${{ steps.cache-key.outputs.value }} + restore-keys: ${{ steps.cache-key.outputs.prefix }} - parallel: - name: Install Composer dependencies - if: steps.cache.outputs.cache-hit != 'true' + if: steps.cache.outputs.cache-matched-key == '' run: composer install --no-interaction --no-progress - name: Install frontend dependencies - if: steps.cache.outputs.cache-hit != 'true' + if: steps.cache.outputs.cache-matched-key == '' # pnpm won't expand env vars in credentials coming from the project # .npmrc, so the Font Awesome token goes in the user-level one. run: | @@ -95,27 +106,44 @@ jobs: CRAFT_FONTAWESOME_TOKEN: ${{ secrets.CRAFT_FONTAWESOME_TOKEN }} - name: Copy icons - if: steps.cache.outputs.cache-hit != 'true' + if: steps.cache.outputs.cache-matched-key == '' run: composer copy-icons # Build the garnish package so its `dist/*.d.ts` exist — `vue-tsc` resolves # `@craftcms/garnish` from `dist` (its `types`), and dist is gitignored. - name: Build @craftcms/garnish - if: steps.cache.outputs.cache-hit != 'true' + if: steps.cache.outputs.cache-matched-key == '' run: vp run build:garnish - name: Build @craftcms/ui - if: steps.cache.outputs.cache-hit != 'true' + if: steps.cache.outputs.cache-matched-key == '' run: vp run build:ui - name: Generate TypeScript types - if: steps.cache.outputs.cache-hit != 'true' + if: steps.cache.outputs.cache-matched-key == '' run: vp run generate:types - name: Generate Wayfinder routes - if: steps.cache.outputs.cache-hit != 'true' + if: steps.cache.outputs.cache-matched-key == '' run: vp run generate:wayfinder + - name: Save install + build + generated cache + if: steps.cache.outputs.cache-matched-key == '' + uses: actions/cache/save@v4 + with: + path: | + node_modules + packages/*/node_modules + packages/craftcms-ui/dist + packages/craftcms-garnish/dist + vendor + cms-assets/resources/icons + resources/js/actions + resources/js/routes + resources/js/wayfinder + resources/js/generated + key: ${{ steps.cache-key.outputs.value }} + - name: Determine deploy branch id: branch run: | @@ -143,8 +171,10 @@ jobs: cache: true run-install: false + # This run's own cache, or the earlier one build-assets matched. A miss + # means nothing is installed, so fail here rather than at the build. - name: Restore install + build + generated cache - uses: actions/cache@v4 + uses: actions/cache/restore@v4 with: path: | node_modules @@ -158,6 +188,8 @@ jobs: resources/js/wayfinder resources/js/generated key: ${{ needs.build-assets.outputs.cache-key }} + restore-keys: ${{ needs.build-assets.outputs.cache-prefix }} + fail-on-cache-miss: true - name: Build Storybook run: vp run @craftcms/ui#build:storybook @@ -190,8 +222,10 @@ jobs: cache: true run-install: false + # This run's own cache, or the earlier one build-assets matched. A miss + # means nothing is installed, so fail here rather than at the build. - name: Restore install + build + generated cache - uses: actions/cache@v4 + uses: actions/cache/restore@v4 with: path: | node_modules @@ -205,6 +239,8 @@ jobs: resources/js/wayfinder resources/js/generated key: ${{ needs.build-assets.outputs.cache-key }} + restore-keys: ${{ needs.build-assets.outputs.cache-prefix }} + fail-on-cache-miss: true - name: Build Storybook # build-assets already ran the transformer and Wayfinder, and their diff --git a/.gitignore b/.gitignore index 7400fcd761a..317957228f4 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ Source/.idea/* Build/* *.sass-cache node_modules/ +.pnpm-store/ vendor/ /bower_components cpresources/* diff --git a/packages/craftcms-legacy/cp/src/js/ElementEditor.js b/packages/craftcms-legacy/cp/src/js/ElementEditor.js index 302b1b009ca..e562ae7f6f3 100644 --- a/packages/craftcms-legacy/cp/src/js/ElementEditor.js +++ b/packages/craftcms-legacy/cp/src/js/ElementEditor.js @@ -1928,7 +1928,8 @@ Craft.ElementEditor = Garnish.Base.extend( } if (this.isFullPage && !this.$contentContainer.length) { - this.$contentContainer = this.settings.$contentContainer ?? $('#content'); + this.$contentContainer = + this.settings.$contentContainer ?? $('#content'); } this.formHost = this.$contentContainer.find( diff --git a/packages/craftcms-ui/src/components/switch-button/switch-button.ts b/packages/craftcms-ui/src/components/switch-button/switch-button.ts index f87fbce1a5a..ee51a5c87d1 100644 --- a/packages/craftcms-ui/src/components/switch-button/switch-button.ts +++ b/packages/craftcms-ui/src/components/switch-button/switch-button.ts @@ -95,7 +95,9 @@ export default class CraftSwitchButton extends LionSwitchButton { :host([checked]) .switch-button__thumb:after { --_checkmark-size: calc(var(--c-switch-thumb-height) / 2); - --_checkmark-offset: calc((var(--c-switch-thumb-height) - var(--_checkmark-size)) / 2 - 1px); + --_checkmark-offset: calc( + (var(--c-switch-thumb-height) - var(--_checkmark-size)) / 2 - 1px + ); content: ''; position: absolute; inset-block-start: var(--_checkmark-offset); diff --git a/resources/js/modules/forms/AdminTableNode.vue b/resources/js/modules/forms/AdminTableNode.vue index 1008b2412d0..dd4145d773e 100644 --- a/resources/js/modules/forms/AdminTableNode.vue +++ b/resources/js/modules/forms/AdminTableNode.vue @@ -12,7 +12,15 @@ useTable, } from '@tanstack/vue-table'; import {StorageSerializers, watchDebounced} from '@vueuse/core'; - import {computed, defineComponent, h, onMounted, ref, watch} from 'vue'; + import { + computed, + defineComponent, + h, + onMounted, + ref, + shallowRef, + watch, + } from 'vue'; import CraftInput from '@craftcms/ui/vue/CraftInput.vue'; import CraftSelectRich, { type SelectRichOption, @@ -20,6 +28,7 @@ import ActionMenu from '@/common/components/ActionMenu.vue'; import CpLink from '@/common/components/CpLink.vue'; import type { + ActionItemButton, ActionItemLink, CheckboxOption, PaginationData, @@ -30,6 +39,7 @@ } from '@/modules/elements/types/actions'; import LayoutSlot from '@/common/components/LayoutSlot.vue'; import {useLocalStorage} from '@/common/composables/useStorage'; + import {openSlideout} from '@/common/slideouts'; import AdminTable from '@/modules/admin-table/components/AdminTable.vue'; import ElementStatus from '@/modules/elements/ElementStatus.vue'; import IndexViewSettings from '@/modules/elements/index/components/IndexViewSettings.vue'; @@ -44,6 +54,7 @@ type CraftTableFeatures, } from '@/modules/admin-table/craftTable'; import AdminTableDeleteModal from './AdminTableDeleteModal.vue'; + import FormModal from './FormModal.vue'; import type {FormNodePayload, FormValues} from './types'; interface TableColumn { @@ -54,12 +65,22 @@ interface TableLink { label: string; - url: string | null; + url?: string | null; + /** Opens the URL's screen in a slideout, reloading the table once it's saved. */ + slideout?: boolean; + } + + /** A menu item that opens a server-built form in a modal instead of following a link. */ + interface TableModalAction { + label: string; + modalUrl: string; + actionUrl: string; + params?: FormValues; } interface TableMenu { label: string; - items: TableLink[]; + items: Array; } interface TableIcon { @@ -559,9 +580,37 @@ } function renderLink(link: TableLink) { - return link.url - ? h(CpLink, {href: link.url, inertia: false}, () => link.label) - : link.label; + if (!link.url) { + return link.label; + } + + const url = link.url; + + return h( + CpLink, + { + href: url, + inertia: false, + // A modified click still follows the link, to open the screen in a tab. + onClick: link.slideout + ? (event: MouseEvent) => { + if ( + event.altKey || + event.ctrlKey || + event.metaKey || + event.shiftKey || + event.button !== 0 + ) { + return; + } + + event.preventDefault(); + void openSlideout(url, {onSaved: refreshTable}); + } + : undefined, + }, + () => link.label + ); } function renderIcon(icon: TableIcon) { @@ -572,25 +621,68 @@ return h('div', {innerHTML: cell.html}); } + function isModalAction( + item: TableLink | TableModalAction + ): item is TableModalAction { + return 'modalUrl' in item && 'actionUrl' in item; + } + + const menuModal = shallowRef(null); + + function onMenuModalSubmitted(): void { + menuModal.value = null; + refreshTable(); + } + function renderMenu(menu: TableMenu) { return h( ActionMenu, { label: menu.label, - actions: menu.items.map( - (item): ActionItemLink => ({ - type: 'link', - href: item.url ?? '#', - label: item.label, - }) + actions: menu.items.map((item): ActionItemButton | ActionItemLink => + isModalAction(item) + ? { + type: 'button', + label: item.label, + onClick: () => { + menuModal.value = item; + }, + } + : { + type: 'link', + href: item.url ?? '#', + label: item.label, + } ), }, { - invoker: () => + // Without the slot `craft-action-menu` has no invoker, and files the button + // away with its content. + // + // The label reads as plain cell text until its row (`DataTable`'s + // `cp-table-row`) is hovered or focused, or its menu is open, so a column of + // these doesn't turn into a column of buttons. Where there's no hover to + // reveal it, the chevron stays put. + invoker: ({attributes}: {attributes: Record}) => h( 'craft-button', - {type: 'button', size: 'small', variant: 'outline'}, - menu.label + { + ...attributes, + type: 'button', + size: 'small', + variant: 'plain', + flush: '', + class: 'group', + }, + [ + menu.label, + h('craft-icon', { + name: 'chevron-down', + slot: 'suffix', + class: + 'transition-opacity [@media(hover:hover)]:opacity-0 [.cp-table-row:hover_&]:opacity-100 [.cp-table-row:focus-within_&]:opacity-100 group-aria-expanded:opacity-100', + }), + ] ), } ); @@ -749,7 +841,9 @@ : []; }, }, - getRowId: (row) => String(row.id), + // Rows only need an `id` to be reordered or deleted, and rows sharing an id + // send `DataTable` into a render loop. + getRowId: (row, index) => String(row.id ?? `row-${index}`), enableRowSelection: (row) => hasBulkFooter.value && row.original._deletable !== false, onRowSelectionChange: (updater) => { @@ -1130,5 +1224,13 @@ @close="deletingRow = null" @deleted="onModalDeleted" /> + diff --git a/resources/js/modules/forms/FormModal.vue b/resources/js/modules/forms/FormModal.vue index 951f1e77ec6..2517b77c6b5 100644 --- a/resources/js/modules/forms/FormModal.vue +++ b/resources/js/modules/forms/FormModal.vue @@ -101,11 +101,8 @@ @close="emit('close')" @submit="submit" > - + + + diff --git a/resources/js/modules/forms/nested-elements/NestedElementsControl.integration.test.ts b/resources/js/modules/forms/nested-elements/NestedElementsControl.integration.test.ts index d5b256a59a4..4ac97262aa8 100644 --- a/resources/js/modules/forms/nested-elements/NestedElementsControl.integration.test.ts +++ b/resources/js/modules/forms/nested-elements/NestedElementsControl.integration.test.ts @@ -272,7 +272,9 @@ describe('NestedElementsControl', () => { expect( indexRequests().filter((request) => Number(request.p ?? 1) === 2) ).toHaveLength(destinationReadsBeforeFailedMove); - }); + // Five page loads and three moves: ~0.4s locally, but ~5s on a shared CI + // runner, right at the default limit. + }, 15_000); it('resets paging after the first create save and a toolbar paste', async () => { const {root, ownerRefresh, pasteElements, submitElement} = mount(); diff --git a/src/Form/Nodes/Table.php b/src/Form/Nodes/Table.php index e32bc56e99b..1f3fc5af47b 100644 --- a/src/Form/Nodes/Table.php +++ b/src/Form/Nodes/Table.php @@ -15,7 +15,11 @@ use function CraftCms\Cms\t; -/** A listing of rows supplied through node props, not form values. */ +/** + * A listing of rows supplied through node props, not form values. + * + * @since 6.0.0 + */ class Table implements Node { use Conditionable; @@ -108,6 +112,15 @@ public function columns(array $columns): static * without sanitization in both renderers. Encode untrusted content with * {@see Html::encode()} before passing it. * + * A link with `'slideout' => true` opens its URL's screen in a slideout, and reloads the + * table once that screen is saved. + * + * A menu item can be `['label' => string, 'modalUrl' => string, 'actionUrl' => string, + * 'params' => ?array]` instead of a link, to open a modal Form. `modalUrl` is requested + * via GET with `params` and must return JSON `{form: FormPayload, title?: string, + * submitLabel?: string}`; submitting posts the Form's values plus `params` to `actionUrl`, + * then reloads the table. + * * `_deletable => false` suppresses deletion of one row. `_status` accepts a * boolean or status string and renders an indicator in the first column. * `_search` overrides client-side search text; otherwise columns' text is used. @@ -135,7 +148,10 @@ public static function prepareRows(array $rows): array return array_map(self::resolveRowStatus(...), $rows); } - /** @param array $row */ + /** + * @param array $row + * @return array + */ private static function resolveRowStatus(array $row): array { if (! array_key_exists('_status', $row) || $row['_status'] === null) { @@ -385,7 +401,7 @@ public static function renderHtml(NodePayload $node, FormPayload $payload, FormH 'class' => ['zilch'], ]); } else { - $renderLink = fn (array $link): string => $link['url'] !== null + $renderLink = fn (array $link): string => ($link['url'] ?? null) !== null ? Html::a(Html::encode($link['label']), $link['url']) : Html::encode($link['label']); diff --git a/src/View/HtmlStack.php b/src/View/HtmlStack.php index afd990d66e6..f14333ea0fe 100644 --- a/src/View/HtmlStack.php +++ b/src/View/HtmlStack.php @@ -766,8 +766,8 @@ public function clearScriptBuffer(): array * * Position-keyed properties (`js`, `scripts`, `jsFiles`, `html`) are merged per-position. * Flat-keyed properties (`cssFiles`, `css`, `jsImports`, `metaTags`, `linkTags`) are - * merged by key, with overwritten entries moved to the end to reflect the latest - * registration order. Icons are deduplicated and appended. + * merged by key, with overwritten entries keeping the position of their first + * registration. Icons are deduplicated and appended. * * @param array $buffer The captured state from [[clearBuffer()]], keyed by property name. */ @@ -896,7 +896,7 @@ private function loadJs(): string } /** - * Keeps overwritten entries in their latest registration order. + * Keeps overwritten entries in the position of their first registration. * * @param array $entries */ diff --git a/tests/Feature/Form/TableNodeMenuModalTest.php b/tests/Feature/Form/TableNodeMenuModalTest.php new file mode 100644 index 00000000000..4c4fa50d9af --- /dev/null +++ b/tests/Feature/Form/TableNodeMenuModalTest.php @@ -0,0 +1,57 @@ + 1, + 'stock' => [ + 'label' => '7', + 'items' => [ + ['label' => 'History', 'url' => 'stock/1/history'], + [ + 'label' => 'Adjust', + 'modalUrl' => 'stock/adjust-modal', + 'actionUrl' => 'stock/adjust', + 'params' => ['id' => 1], + ], + ], + ], + ]]; +} + +it('passes menu items that open a modal Form through to the table', function () { + $props = Table::make('stock') + ->columns([['key' => 'stock', 'label' => 'Stock']]) + ->rows(menuModalRows()) + ->props(); + + expect($props['rows'][0]['stock']['items'][1])->toBe([ + 'label' => 'Adjust', + 'modalUrl' => 'stock/adjust-modal', + 'actionUrl' => 'stock/adjust', + 'params' => ['id' => 1], + ]); +}); + +it('renders menu items that open a modal Form as plain labels without JavaScript', function () { + $payload = app(FormResolver::class)->resolve(Form::make([ + Table::make('stock') + ->columns([['key' => 'stock', 'label' => 'Stock']]) + ->rows(menuModalRows()), + ]), new FormContext); + + $cell = new Crawler(app(FormHtmlRenderer::class)->render($payload))->filter('tbody td'); + + expect($cell->text())->toBe('History, Adjust') + ->and($cell->filter('a'))->toHaveCount(1) + ->and($cell->filter('a')->attr('href'))->toEndWith('stock/1/history'); +}); diff --git a/tests/Unit/View/HtmlStackTest.php b/tests/Unit/View/HtmlStackTest.php index 21587827287..bd6eee9a998 100644 --- a/tests/Unit/View/HtmlStackTest.php +++ b/tests/Unit/View/HtmlStackTest.php @@ -27,7 +27,11 @@ public function register(HtmlStack $htmlStack): void } } -function assertLatestRegistrationOrder(string $output): void +/** + * The re-registered `target` key takes its latest value but keeps the slot of its + * first registration, ahead of `middle`, so dependency order isn't disturbed. + */ +function assertFirstRegistrationOrder(string $output): void { $firstPos = strpos($output, 'first'); $middlePos = strpos($output, 'middle'); @@ -35,8 +39,8 @@ function assertLatestRegistrationOrder(string $output): void expect($output) ->not->toContain('initial') - ->and($firstPos)->toBeLessThan($middlePos) - ->and($middlePos)->toBeLessThan($updatedPos); + ->and($firstPos)->toBeLessThan($updatedPos) + ->and($updatedPos)->toBeLessThan($middlePos); } dataset('ordered asset cases', [ @@ -438,8 +442,8 @@ function assertLatestRegistrationOrder(string $output): void }); -describe('latest registration order', function () { - it('moves keyed registrations to their latest registration order', function (array $case) { +describe('keyed registration order', function () { + it('keeps re-registered keys in their first registration position', function (array $case) { $case['register']($this->registry, 'first', 'first'); $case['register']($this->registry, 'initial', 'target'); $case['register']($this->registry, 'middle', 'middle'); @@ -447,7 +451,7 @@ function assertLatestRegistrationOrder(string $output): void $output = $case['render']($this->registry); - assertLatestRegistrationOrder($output); + assertFirstRegistrationOrder($output); })->with('ordered asset cases'); }); @@ -753,7 +757,7 @@ function assertLatestRegistrationOrder(string $output): void ->toContain('.buffered'); }); - it('preserves latest keyed registration order when applying a buffer', function (array $case) { + it('keeps re-registered keys in their first registration position when applying a buffer', function (array $case) { $case['register']($this->registry, 'first', 'first'); $case['register']($this->registry, 'initial', 'target'); @@ -766,7 +770,7 @@ function assertLatestRegistrationOrder(string $output): void $output = $case['render']($this->registry); - assertLatestRegistrationOrder($output); + assertFirstRegistrationOrder($output); })->with('ordered asset cases'); it('handles partial buffer state', function () {