From 9da236b0e3964d91aca37192f899cd66b495c5fb Mon Sep 17 00:00:00 2001 From: Jan-Niclas Loosen Date: Sat, 26 Sep 2026 18:21:03 +0200 Subject: [PATCH] refactor: ColumnWidths holds the column-width rules (H5) Composer::row uses ColumnWidths::parse; rowWidthsError and its error-state save/restore are gone. The update width check moves out of DiviUpdateAbility into ColumnWidths::updateErrors. Behaviour unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7 --- .../Divi/Compose/ColumnWidths.php | 214 ++++++++++++++++++ src/Integrations/Divi/Compose/Composer.php | 125 +--------- .../Divi/Pages/DiviUpdateAbility.php | 91 +------- .../Divi/Compose/ColumnWidthsTest.php | 43 ++++ .../Divi/Compose/ComposerTest.php | 13 -- 5 files changed, 267 insertions(+), 219 deletions(-) create mode 100644 src/Integrations/Divi/Compose/ColumnWidths.php create mode 100644 tests/Unit/Integrations/Divi/Compose/ColumnWidthsTest.php diff --git a/src/Integrations/Divi/Compose/ColumnWidths.php b/src/Integrations/Divi/Compose/ColumnWidths.php new file mode 100644 index 0000000..173afa9 --- /dev/null +++ b/src/Integrations/Divi/Compose/ColumnWidths.php @@ -0,0 +1,214 @@ +, error: ?string} + */ + public static function parse(mixed $columns): array + { + $parts = is_string($columns) ? array_map('trim', explode(',', $columns)) : []; + $widths = []; + $fractions = []; + foreach ($parts as $part) { + $valid = preg_match('/^(\d{1,2})_(\d{1,2})$/D', $part, $match) === 1 + && (int) $match[1] >= 1 + && (int) $match[1] <= (int) $match[2]; + if (! $valid) { + return self::failed(self::FORMAT); + } + + [$numerator, $denominator] = [(int) $match[1], (int) $match[2]]; + if (24 % $denominator === 0) { + $widths[] = ($numerator * 24 / $denominator) . '_24'; + } elseif (in_array($part, self::SPECIAL_WIDTHS, true)) { + $widths[] = $part; + } else { + return self::failed(sprintf('Spaltenbreite %s gibt es in Divi nicht', $part)); + } + $fractions[] = [$numerator, $denominator]; + } + + if ($widths === [] || ! self::sumsToOne($fractions)) { + return self::failed(self::FULL_ROW); + } + + return ['widths' => $widths, 'error' => null]; + } + + /** + * Ruling S41: the desktop flexType values of all columns of a row (in order), checked with + * the rules of "columns" – valid Divi widths that fill the row exactly. Null when valid, + * else the German problem. + * + * @param list $flexTypes + */ + public static function rowError(array $flexTypes): ?string + { + $valid = $flexTypes !== [] && array_filter( + $flexTypes, + static fn (mixed $width): bool => is_string($width) && preg_match('/^\d{1,2}_\d{1,2}$/D', $width) === 1 + ) === $flexTypes; + + return $valid ? self::parse(implode(',', $flexTypes))['error'] : 'Spaltenbreiten wie "8_24" erwartet'; + } + + /** Ruling S43: whether $width is a column class the Visual Builder offers. */ + public static function isColumnClass(mixed $width): bool + { + if (! is_string($width)) { + return false; + } + + return in_array($width, self::COLUMN_CLASSES, true) + || (preg_match('/^([1-9]|1\d|2[0-4])_24$/D', $width) === 1); + } + + /** + * Column widths on update (rulings S41, S43). Every new desktop flexType must be one of the + * builder's column classes (isColumnClass()) or null (removed: Divi's default 24_24). The + * row sum is only checked for rows that are exact today – every column has a class other + * than "none", the classes fill the row (rowError()), the row does not wrap or use a grid + * on desktop – and when no new width is "none". Such a row must stay exact after all + * changes; other rows are laid out by Divi as the builder allows. + * + * @param list, 2: string}> $updates Block, its new attrs, input path. + * @return array + */ + public static function updateErrors(NodeIndex $index, array $updates): array + { + $errors = []; + $new = []; + $rows = []; + foreach ($updates as [$block, $attrs, $field]) { + $width = self::width($attrs); + if ($block->name() !== 'divi/column' || $width === self::width($block->attrs())) { + continue; + } + + if ($width !== null && ! self::isColumnClass($width)) { + $errors[$field . '.attrs.' . self::SIZING] = 'Spaltenbreite (flexType) wie im Divi-Builder erwartet: ' + . '1_24 … 24_24, 1_5, 2_5, 3_5, 1_7, 1_9, 1_10, 1_11 oder none'; + continue; + } + + $new[spl_object_id($block)] = $width ?? self::DEFAULT_WIDTH; + $row = $index->parentOf($block); + if ($row instanceof Block) { + $rows[spl_object_id($row)]['row'] = $row; + $rows[spl_object_id($row)]['fields'][] = $field; + } + } + + foreach ($rows as ['row' => $row, 'fields' => $fields]) { + $before = []; + $after = []; + foreach ($row->children() as $column) { + $before[] = self::width($column->attrs()); + $after[] = $new[spl_object_id($column)] ?? self::width($column->attrs()); + } + + if (! self::isExact($row, $before) || in_array('none', $after, true)) { + continue; + } + + $error = self::rowError($after); + if ($error !== null) { + foreach ($fields as $field) { + $errors[$field . '.attrs.' . self::SIZING] = $error + . ' (Breiten aller Spalten der Zeile zusammen ändern)'; + } + } + } + + return $errors; + } + + /** @param list $widths Desktop flexTypes of the row's columns today. */ + private static function isExact(Block $row, array $widths): bool + { + $layout = AttrMap::desktop($row->attrs(), 'module.decoration.layout'); + $wraps = is_array($layout) + && (($layout['flexWrap'] ?? null) === 'wrap' || ($layout['display'] ?? null) === 'grid'); + if ($wraps) { + return false; + } + + return self::rowError($widths) === null; + } + + /** @param array $attrs */ + private static function width(array $attrs): mixed + { + $sizing = AttrMap::desktop($attrs, self::SIZING); + + return is_array($sizing) ? ($sizing['flexType'] ?? null) : null; + } + + /** @return array{widths: list, error: string} */ + private static function failed(string $error): array + { + return ['widths' => [self::DEFAULT_WIDTH], 'error' => $error]; + } + + /** + * Whether the fractions add up to exactly 1: numerators scaled to the least common + * denominator (denominators are divisors of 24 or 5, 7, 9, 10, 11 – at most 27720). + * + * @param list $fractions [numerator, denominator] + */ + private static function sumsToOne(array $fractions): bool + { + $common = 1; + foreach ($fractions as [, $denominator]) { + $common = intdiv($common * $denominator, self::gcd($common, $denominator)); + } + + $sum = 0; + foreach ($fractions as [$numerator, $denominator]) { + $sum += $numerator * intdiv($common, $denominator); + } + + return $sum === $common; + } + + private static function gcd(int $a, int $b): int + { + return $b === 0 ? $a : self::gcd($b, $a % $b); + } +} diff --git a/src/Integrations/Divi/Compose/Composer.php b/src/Integrations/Divi/Compose/Composer.php index a6bfd32..149f107 100644 --- a/src/Integrations/Divi/Compose/Composer.php +++ b/src/Integrations/Divi/Compose/Composer.php @@ -24,8 +24,9 @@ use LIT\MCP\Integrations\Divi\Schema\ShortformMap; * preset ids. A value that fails there is an error, never stored raw (S13). What Composer * writes itself is not client text: the builder version (checked in the constructor), block * names (catalog + StructureRules + Block::create), "fullwidth" and column flexTypes built - * from digits. Design-system names that match several entries are errors listing the ids - * (ruling S13a: colors/variables in Placeholders, presets in DesignSystem::modulePresetId()). + * from digits (ColumnWidths). Design-system names that match several entries are errors + * listing the ids (ruling S13a: colors/variables in Placeholders, presets in + * DesignSystem::modulePresetId()). */ final class Composer { @@ -36,9 +37,6 @@ final class Composer /** Keys of one lit-divi-update change ("id" names the node, read by the ability). */ private const UPDATE = ['id', 'set', 'attrs', 'preset', 'label']; - /** Column fractions Divi's flex layout knows besides multiples of 1/24. */ - private const SPECIAL_WIDTHS = ['1_5', '2_5', '3_5', '4_5', '1_7', '1_9', '1_10', '1_11']; - /** ET_BUILDER_PRODUCT_VERSION, e.g. "5.2.1" – written into every new block. */ private const VERSION = '/^[0-9][0-9A-Za-z.+-]*$/D'; @@ -48,12 +46,6 @@ final class Composer private const DEFAULT_LEVEL = 'h2'; - /** - * The builder's "Column Class" choices (Divi 5.2.1 visual-builder/build/field-library.js): - * n_24, the special fractions and "none" (no width class). Ruling S43. - */ - private const COLUMN_CLASSES = ['1_5', '2_5', '3_5', '1_7', '1_9', '1_10', '1_11', 'none']; - /** Advice of the S40 overlap error for new nodes. */ private const ONE_FORM = 'für diesen Pfad nur Kurzform oder attrs angeben'; @@ -342,7 +334,11 @@ final class Composer private function row(array $spec, string $path, string $context): Block { $this->unknownKeys($spec, array_merge(self::COMMON, self::ROW), $path); - $widths = $this->widths($spec['columns'] ?? '4_4', $path . '.columns'); + $parsed = ColumnWidths::parse($spec['columns'] ?? '4_4'); + if ($parsed['error'] !== null) { + $this->errors[$path . '.columns'] = $parsed['error']; + } + $widths = $parsed['widths']; $label = $context === '' ? null : $context . ' – Zeile'; $attrs = $this->common('divi/row', ['builderVersion' => $this->builderVersion], $spec, $path, $label); $row = Block::create('divi/row', $attrs); @@ -604,111 +600,6 @@ final class Composer } } - /** - * "1_3,2_3" → flexTypes ["8_24", "16_24"]; the fractions must add up to exactly one row - * (exact integer arithmetic over the common denominator, ruling S34 – no float tolerance). - * - * @return list - */ - private function widths(mixed $columns, string $path): array - { - $parts = is_string($columns) ? array_map('trim', explode(',', $columns)) : []; - $widths = []; - $fractions = []; - foreach ($parts as $part) { - $valid = preg_match('/^(\d{1,2})_(\d{1,2})$/D', $part, $match) === 1 - && (int) $match[1] >= 1 - && (int) $match[1] <= (int) $match[2]; - if (! $valid) { - $this->errors[$path] = 'Spaltenaufteilung wie "1_2,1_2" oder "1_3,2_3" erwartet'; - - return ['24_24']; - } - - [$numerator, $denominator] = [(int) $match[1], (int) $match[2]]; - if (24 % $denominator === 0) { - $widths[] = ($numerator * 24 / $denominator) . '_24'; - } elseif (in_array($part, self::SPECIAL_WIDTHS, true)) { - $widths[] = $part; - } else { - $this->errors[$path] = sprintf('Spaltenbreite %s gibt es in Divi nicht', $part); - - return ['24_24']; - } - $fractions[] = [$numerator, $denominator]; - } - - if ($widths === [] || ! self::sumsToOne($fractions)) { - $this->errors[$path] = 'die Spaltenbreiten müssen zusammen eine volle Zeile ergeben (z. B. "1_3,2_3")'; - - return ['24_24']; - } - - return $widths; - } - - /** - * Ruling S41: the desktop flexType values of all columns of a row (in order), checked with - * the rules of "columns" – valid Divi widths that fill the row exactly. Null when valid, - * else the German problem. - * - * @param list $flexTypes - */ - public function rowWidthsError(array $flexTypes): ?string - { - $saved = $this->errors; - $this->errors = []; - $valid = $flexTypes !== [] && array_filter( - $flexTypes, - static fn (mixed $width): bool => is_string($width) && preg_match('/^\d{1,2}_\d{1,2}$/D', $width) === 1 - ) === $flexTypes; - if ($valid) { - $this->widths(implode(',', $flexTypes), 'w'); - } - - $error = $valid ? ($this->errors['w'] ?? null) : 'Spaltenbreiten wie "8_24" erwartet'; - $this->errors = $saved; - - return $error; - } - - /** Ruling S43: whether $width is a column class the Visual Builder offers. */ - public static function isColumnClass(mixed $width): bool - { - if (! is_string($width)) { - return false; - } - - return in_array($width, self::COLUMN_CLASSES, true) - || (preg_match('/^([1-9]|1\d|2[0-4])_24$/D', $width) === 1); - } - - /** - * Whether the fractions add up to exactly 1: numerators scaled to the least common - * denominator (denominators are divisors of 24 or 5, 7, 9, 10, 11 – at most 27720). - * - * @param list $fractions [numerator, denominator] - */ - private static function sumsToOne(array $fractions): bool - { - $common = 1; - foreach ($fractions as [, $denominator]) { - $common = intdiv($common * $denominator, self::gcd($common, $denominator)); - } - - $sum = 0; - foreach ($fractions as [$numerator, $denominator]) { - $sum += $numerator * intdiv($common, $denominator); - } - - return $sum === $common; - } - - private static function gcd(int $a, int $b): int - { - return $b === 0 ? $a : self::gcd($b, $a % $b); - } - /** * @param array $spec * @param list $allowed diff --git a/src/Integrations/Divi/Pages/DiviUpdateAbility.php b/src/Integrations/Divi/Pages/DiviUpdateAbility.php index 90913c0..52ded7a 100644 --- a/src/Integrations/Divi/Pages/DiviUpdateAbility.php +++ b/src/Integrations/Divi/Pages/DiviUpdateAbility.php @@ -7,9 +7,8 @@ namespace LIT\MCP\Integrations\Divi\Pages; use LIT\MCP\Abilities\Ability; use LIT\MCP\Abilities\Errors; use LIT\MCP\Abilities\ToolResult; -use LIT\MCP\Integrations\Divi\BlockTree\AttrMap; use LIT\MCP\Integrations\Divi\BlockTree\Block; -use LIT\MCP\Integrations\Divi\Compose\Composer; +use LIT\MCP\Integrations\Divi\Compose\ColumnWidths; use WP_Error; /** @@ -22,11 +21,6 @@ use WP_Error; */ final class DiviUpdateAbility implements Ability { - private const SIZING = 'module.decoration.sizing'; - - /** Divi's column default (column/module-default-render-attributes.json). */ - private const DEFAULT_WIDTH = '24_24'; - public function __construct(private DiviWriter $writer) { } @@ -161,7 +155,7 @@ final class DiviUpdateAbility implements Ability } if ($errors === []) { - $errors = $this->rowWidths($page, $composer, $updates); + $errors = ColumnWidths::updateErrors($page->index, $updates); } if ($errors !== []) { @@ -183,85 +177,4 @@ final class DiviUpdateAbility implements Ability return $this->writer->save($this->name(), $page, $change, $input); } - - /** - * Column widths on update (rulings S41, S43). Every new desktop flexType must be one of the - * builder's column classes (Composer::isColumnClass()) or null (removed: Divi's default - * 24_24). The row sum is only checked for rows that are exact today – every column has a - * class other than "none", the classes fill the row (Composer::rowWidthsError()), the row - * does not wrap or use a grid on desktop – and when no new width is "none". Such a row - * must stay exact after all changes; other rows are laid out by Divi as the builder allows. - * - * @param list, 2: string}> $updates - * @return array - */ - private function rowWidths(DiviPage $page, Composer $composer, array $updates): array - { - $errors = []; - $new = []; - $rows = []; - foreach ($updates as [$block, $attrs, $field]) { - $width = self::width($attrs); - if ($block->name() !== 'divi/column' || $width === self::width($block->attrs())) { - continue; - } - - if ($width !== null && ! Composer::isColumnClass($width)) { - $errors[$field . '.attrs.' . self::SIZING] = 'Spaltenbreite (flexType) wie im Divi-Builder erwartet: ' - . '1_24 … 24_24, 1_5, 2_5, 3_5, 1_7, 1_9, 1_10, 1_11 oder none'; - continue; - } - - $new[spl_object_id($block)] = $width ?? self::DEFAULT_WIDTH; - $row = $page->index->parentOf($block); - if ($row instanceof Block) { - $rows[spl_object_id($row)]['row'] = $row; - $rows[spl_object_id($row)]['fields'][] = $field; - } - } - - foreach ($rows as ['row' => $row, 'fields' => $fields]) { - $before = []; - $after = []; - foreach ($row->children() as $column) { - $before[] = self::width($column->attrs()); - $after[] = $new[spl_object_id($column)] ?? self::width($column->attrs()); - } - - if (! self::isExact($row, $before, $composer) || in_array('none', $after, true)) { - continue; - } - - $error = $composer->rowWidthsError($after); - if ($error !== null) { - foreach ($fields as $field) { - $errors[$field . '.attrs.' . self::SIZING] = $error - . ' (Breiten aller Spalten der Zeile zusammen ändern)'; - } - } - } - - return $errors; - } - - /** @param list $widths Desktop flexTypes of the row's columns today. */ - private static function isExact(Block $row, array $widths, Composer $composer): bool - { - $layout = AttrMap::desktop($row->attrs(), 'module.decoration.layout'); - $wraps = is_array($layout) - && (($layout['flexWrap'] ?? null) === 'wrap' || ($layout['display'] ?? null) === 'grid'); - if ($wraps) { - return false; - } - - return $composer->rowWidthsError($widths) === null; - } - - /** @param array $attrs */ - private static function width(array $attrs): mixed - { - $sizing = AttrMap::desktop($attrs, self::SIZING); - - return is_array($sizing) ? ($sizing['flexType'] ?? null) : null; - } } diff --git a/tests/Unit/Integrations/Divi/Compose/ColumnWidthsTest.php b/tests/Unit/Integrations/Divi/Compose/ColumnWidthsTest.php new file mode 100644 index 0000000..8cb4e22 --- /dev/null +++ b/tests/Unit/Integrations/Divi/Compose/ColumnWidthsTest.php @@ -0,0 +1,43 @@ + ['8_24', '16_24'], 'error' => null], ColumnWidths::parse('1_3, 2_3')); + self::assertSame(['24_24'], ColumnWidths::parse(42)['widths']); + self::assertStringContainsString('erwartet', (string) ColumnWidths::parse('abc')['error']); + self::assertStringContainsString('1_13', (string) ColumnWidths::parse('1_13,12_13')['error']); + self::assertStringContainsString('volle Zeile', (string) ColumnWidths::parse('1_2,1_3')['error']); + } + + /** Ruling S43: the builder's column classes. */ + public function testColumnClassesAreTheBuilderList(): void + { + foreach (['1_24', '24_24', '1_5', '3_5', '1_11', 'none'] as $class) { + self::assertTrue(ColumnWidths::isColumnClass($class), $class); + } + + foreach (['0_24', '25_24', '1_2', '4_5', '1_13', '', null, 8] as $class) { + self::assertFalse(ColumnWidths::isColumnClass($class), var_export($class, true)); + } + } +} diff --git a/tests/Unit/Integrations/Divi/Compose/ComposerTest.php b/tests/Unit/Integrations/Divi/Compose/ComposerTest.php index 155b441..d20390e 100644 --- a/tests/Unit/Integrations/Divi/Compose/ComposerTest.php +++ b/tests/Unit/Integrations/Divi/Compose/ComposerTest.php @@ -298,19 +298,6 @@ final class ComposerTest extends TestCase self::assertSame('h3', $explicit['blocks'][0]->attrs()['title']['decoration']['font']['font']['desktop']['value']['headingLevel']); } - /** Ruling S41: column widths on update follow the rules of "columns". */ - public function testRowWidthsAreChecked(): void - { - $composer = ComposeFixture::composer(); - - self::assertNull($composer->rowWidthsError(['8_24', '16_24'])); - self::assertNull($composer->rowWidthsError(['1_5', '1_5', '1_5', '1_5', '1_5'])); - self::assertNotNull($composer->rowWidthsError(['12_24', '16_24'])); - self::assertNotNull($composer->rowWidthsError(['1_13', '12_13'])); - self::assertNotNull($composer->rowWidthsError(['x', '24_24'])); - self::assertNotNull($composer->rowWidthsError([])); - } - /** Ruling S13: labels go through the choke point; a label that fails is an error, never stored raw. */ public function testLabelsAreCleanedOrReported(): void {