Separated the configuration into form, widget and TUI-runtime layers.#56
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 5 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (139)
📝 WalkthroughWalkthroughThe pull request replaces the Config model with immutable ChangesForm model and runtime migration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This comment has been minimized.
This comment has been minimized.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #56 +/- ##
==========================================
- Coverage 97.48% 97.48% -0.01%
==========================================
Files 97 97
Lines 2863 2858 -5
==========================================
- Hits 2791 2786 -5
Misses 72 72 ☔ View full report in Codecov by Harness. |
|
🚀 Deployed on https://6a58458e78f054c1f0f548ac--tui-docs.netlify.app |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/content/index.mdx`:
- Line 55: Update the Quick Start documentation around the Tui constructor
example to replace the stale config() accessor reference with form() and
appearance(), reflecting the current API while preserving the surrounding
guidance.
In `@src/Derive/Derive.php`:
- Line 32: Replace the double-quoted string literals with single-quoted literals
at src/Derive/Derive.php lines 32-32 for the sprintf format string,
tests/phpunit/Unit/Engine/EngineNonInteractiveTest.php lines 55-55 for the JSON
fixture, and tests/phpunit/Unit/Schema/SchemaGeneratorTest.php lines 173-187 for
the JSON assertion fragments; preserve their existing contents and behavior.
In `@src/Testing/TuiTester.php`:
- Around line 90-100: Update the constructor’s $appearance PHPDoc to state that
NULL uses the builder’s own appearance when $form is a Form, and a default
Appearance when $form is already a FormDefinition. Leave the constructor
implementation unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7a9697d9-0704-4b43-9c31-07e3c7de92cb
⛔ Files ignored due to path filters (2)
docs/architecture/architecture-dark.svgis excluded by!**/*.svgdocs/architecture/architecture.svgis excluded by!**/*.svg
📒 Files selected for processing (109)
README.mddocs/architecture/README.mddocs/architecture/architecture.pumldocs/content/architecture.mdxdocs/content/field-behaviour.mdxdocs/content/headless-collection.mdxdocs/content/index.mdxdocs/content/installation.mdxdocs/content/key-bindings.mdxdocs/content/widgets/calendar.mdxplayground/2-custom-theme/OceanTheme.phpplayground/4-nested-panels/run.phpplayground/8-key-bindings/run.phpsrc/Answers/Answer.phpsrc/Answers/Answers.phpsrc/Answers/SummaryFormatter.phpsrc/Builder/FieldBuilder.phpsrc/Builder/Form.phpsrc/Builder/PanelBuilder.phpsrc/Config/ConfigException.phpsrc/Derive/Derive.phpsrc/Engine/Engine.phpsrc/Input/DefaultKeyMap.phpsrc/Input/KeyMap.phpsrc/Input/KeyMapManager.phpsrc/Input/Scope.phpsrc/Input/VimKeyMap.phpsrc/Model/Appearance.phpsrc/Model/DateBounds.phpsrc/Model/Field.phpsrc/Model/FieldType.phpsrc/Model/FilePickerMode.phpsrc/Model/Fixup.phpsrc/Model/FormDefinition.phpsrc/Model/FormException.phpsrc/Model/NumberBounds.phpsrc/Model/Option.phpsrc/Model/OptionKind.phpsrc/Model/Panel.phpsrc/Model/RenderMode.phpsrc/Model/Weekday.phpsrc/Render/Navigator.phpsrc/Render/PanelController.phpsrc/Resolver/InputResolver.phpsrc/Schema/AgentHelp.phpsrc/Schema/SchemaGenerator.phpsrc/Schema/SchemaValidator.phpsrc/Testing/TuiTester.phpsrc/Theme/DefaultTheme.phpsrc/Theme/ThemeManager.phpsrc/Tui.phpsrc/Widget/CalendarWidget.phpsrc/Widget/Capability/FilterCapableTrait.phpsrc/Widget/Capability/MultiSelectionCapableTrait.phpsrc/Widget/Capability/OptionsCapableInterface.phpsrc/Widget/Capability/OptionsCapableTrait.phpsrc/Widget/Capability/SearchCapableTrait.phpsrc/Widget/Capability/SelectionCapableTrait.phpsrc/Widget/ConfirmWidget.phpsrc/Widget/FilePickerWidget.phpsrc/Widget/Matcher.phpsrc/Widget/MultiSearchWidget.phpsrc/Widget/MultiSelectWidget.phpsrc/Widget/NumberWidget.phpsrc/Widget/PasswordWidget.phpsrc/Widget/PauseWidget.phpsrc/Widget/ReorderWidget.phpsrc/Widget/SearchWidget.phpsrc/Widget/SelectWidget.phpsrc/Widget/TextWidget.phpsrc/Widget/TextareaWidget.phpsrc/Widget/ToggleWidget.phpsrc/Widget/WidgetFactory.phptests/phpunit/Traits/MixedOptionsTrait.phptests/phpunit/Unit/Answers/AnswersTest.phptests/phpunit/Unit/Answers/SummaryFormatterTest.phptests/phpunit/Unit/Builder/FormTest.phptests/phpunit/Unit/Derive/DeriveTest.phptests/phpunit/Unit/Engine/EngineAnswersTest.phptests/phpunit/Unit/Engine/EngineConditionalTest.phptests/phpunit/Unit/Engine/EngineDeclaredBehaviourTest.phptests/phpunit/Unit/Engine/EngineDeriveTest.phptests/phpunit/Unit/Engine/EngineNonInteractiveTest.phptests/phpunit/Unit/Engine/EngineTest.phptests/phpunit/Unit/Input/KeyMapTest.phptests/phpunit/Unit/Model/AppearanceTest.phptests/phpunit/Unit/Model/BuiltModelTest.phptests/phpunit/Unit/Model/DateBoundsTest.phptests/phpunit/Unit/Model/FormDefinitionTest.phptests/phpunit/Unit/Model/NumberBoundsTest.phptests/phpunit/Unit/Model/OptionTest.phptests/phpunit/Unit/Model/WeekdayTest.phptests/phpunit/Unit/Render/NavigatorTest.phptests/phpunit/Unit/Render/PanelControllerTest.phptests/phpunit/Unit/Resolver/InputResolverTest.phptests/phpunit/Unit/Schema/AgentHelpTest.phptests/phpunit/Unit/Schema/SchemaGeneratorTest.phptests/phpunit/Unit/Schema/SchemaValidatorTest.phptests/phpunit/Unit/Testing/AllWidgetsFormTest.phptests/phpunit/Unit/Theme/ThemeOptionsTest.phptests/phpunit/Unit/Theme/ThemeRenderTest.phptests/phpunit/Unit/Translation/TranslationRenderTest.phptests/phpunit/Unit/TuiTest.phptests/phpunit/Unit/Widget/CalendarWidgetTest.phptests/phpunit/Unit/Widget/FilePickerWidgetTest.phptests/phpunit/Unit/Widget/MatcherTest.phptests/phpunit/Unit/Widget/NumberWidgetTest.phptests/phpunit/Unit/Widget/SelectWidgetTest.phptests/phpunit/Unit/Widget/WidgetFactoryTest.php
💤 Files with no reviewable changes (1)
- src/Config/ConfigException.php
49fef06 to
00289dd
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
docs/content/themes.mdx (1)
22-26: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDescribe auto as display-mode detection, not named-theme selection.
''and'auto'resolve to thedefaulttheme; terminal probing determines its light/dark mode.ThemeManageronly registersdefault, so the documented->theme('dark')and->theme('light')calls fail unless consumers register those aliases. Update this paragraph and the surrounding examples accordingly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/content/themes.mdx` around lines 22 - 26, Update the themes documentation paragraph and surrounding examples to describe empty and 'auto' values as resolving to the registered 'default' theme, with terminal probing determining its light/dark display mode. Remove claims that explicit ->theme('dark') or ->theme('light') are built-in opt-outs, and clarify those names require consumer-registered aliases.playground/2-custom-theme/run.php (1)
32-33: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the stale theme-ownership comments.
The code correctly moves
OceanTheme::classtoTui, but the file header and therequirecomment still say the form names the theme. Update those comments so this example consistently documents the new API.Proposed wording
- * Custom theme example: the form names a theme class; run it via the facade. + * Custom theme example: the Tui facade names a theme class and runs the form. -// The require makes the class loadable; the form names it directly, so no +// The require makes the class loadable; the facade names it directly, so noAlso applies to: 45-46
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@playground/2-custom-theme/run.php` around lines 32 - 33, Update the stale comments in the file header and around the require statement to state that OceanTheme is owned or provided by Tui, matching the existing OceanTheme::class move. Leave the Form::create usage and other code unchanged.playground/README.md (2)
136-148: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign the custom-theme example with the facade API.
This new section correctly documents
Tui::theme(...), but the earlier2-custom-themeentry still saysOceanTheme.phpis named directly on the form. Update that entry so the README does not give contradictory instructions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@playground/README.md` around lines 136 - 148, Update the earlier 2-custom-theme README entry to show configuring the theme through the Tui facade’s theme(...) API instead of naming OceanTheme.php directly on the form, keeping the example consistent with the “How the TUI picks a theme” section.
164-166: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the key-map validation timing.
Tui::keys()callsKeyMapManager::create()immediately, so invalid bindings throw when->keys(...)is invoked—not when the form is built. The current wording can mislead users about where to catch configuration errors.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@playground/README.md` around lines 164 - 166, Update the key-map validation wording in the README to state that invalid or conflicting bindings are detected when Tui::keys() invokes KeyMapManager::create(), at the ->keys(...) call, rather than when the form is built or during a session.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/content/configuration.mdx`:
- Around line 42-50: Update the configuration documentation around the Tui
facade description to replace the claim that its runtime settings are shared by
every form with wording that scopes them to the current Tui run. Preserve the
listed options and their existing behavior descriptions.
In `@docs/content/headless-collection.mdx`:
- Around line 19-21: Update the `new Tui` constructor example in the `envPrefix`
documentation to use the current form variable, such as `$form` or
`$formDefinition`, instead of the undefined `$config`; keep the surrounding
prefix behavior explanation unchanged.
In `@docs/content/themes.mdx`:
- Around line 43-46: Update the custom-theme PHP snippet around the Tui theme
call so it is self-contained and copyable: use the namespace where OceanTheme is
declared and import or fully qualify Tui within the snippet. Keep the existing
theme assignment behavior unchanged.
In `@src/Builder/Form.php`:
- Around line 187-197: Complete the FormDefinition/Appearance split: in
src/Builder/Form.php lines 187-197, remove banner and button presentation state
from FormDefinition and provide it through Appearance; in
src/Testing/TuiTester.php lines 89-97, accept and forward an optional
Appearance. Move chrome-default assertions from
src/tests/phpunit/Unit/Model/BuiltModelTest.php lines 93-103 into
Appearance-focused tests, and update docs/architecture/architecture.puml lines
16-17 plus docs/content/architecture.mdx lines 30 and 41-43 to define Appearance
as the presentation/runtime model with its Tui ownership and dependencies.
In `@src/Schema/AgentHelp.php`:
- Around line 23-24: Update the `@param` documentation for the FormDefinition
parameter in AgentHelp to describe it as “The form definition to describe”
instead of referring to it as configuration.
In `@src/Tui.php`:
- Around line 120-122: Update the execution entry points run(), collect(), and
controller() to activate this facade’s translator via
Translator::setShared($this->translator) at the start of each operation,
including when the value is NULL. Remove reliance on constructor-time or
configuration-time shared state so every operation restores the owning Tui
instance’s translator before proceeding.
---
Outside diff comments:
In `@docs/content/themes.mdx`:
- Around line 22-26: Update the themes documentation paragraph and surrounding
examples to describe empty and 'auto' values as resolving to the registered
'default' theme, with terminal probing determining its light/dark display mode.
Remove claims that explicit ->theme('dark') or ->theme('light') are built-in
opt-outs, and clarify those names require consumer-registered aliases.
In `@playground/2-custom-theme/run.php`:
- Around line 32-33: Update the stale comments in the file header and around the
require statement to state that OceanTheme is owned or provided by Tui, matching
the existing OceanTheme::class move. Leave the Form::create usage and other code
unchanged.
In `@playground/README.md`:
- Around line 136-148: Update the earlier 2-custom-theme README entry to show
configuring the theme through the Tui facade’s theme(...) API instead of naming
OceanTheme.php directly on the form, keeping the example consistent with the
“How the TUI picks a theme” section.
- Around line 164-166: Update the key-map validation wording in the README to
state that invalid or conflicting bindings are detected when Tui::keys() invokes
KeyMapManager::create(), at the ->keys(...) call, rather than when the form is
built or during a session.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ba00bfff-08e3-4204-ba91-2be91b26ee82
⛔ Files ignored due to path filters (2)
docs/architecture/architecture-dark.svgis excluded by!**/*.svgdocs/architecture/architecture.svgis excluded by!**/*.svg
📒 Files selected for processing (138)
README.mddocs/architecture/README.mddocs/architecture/architecture.pumldocs/content/architecture.mdxdocs/content/configuration.mdxdocs/content/display-modes.mdxdocs/content/field-behaviour.mdxdocs/content/headless-collection.mdxdocs/content/index.mdxdocs/content/installation.mdxdocs/content/key-bindings.mdxdocs/content/panels.mdxdocs/content/themes.mdxdocs/content/translations.mdxdocs/content/widgets/calendar.mdxplayground/10-field-styles/run.phpplayground/2-custom-theme/OceanTheme.phpplayground/2-custom-theme/run.phpplayground/3-widgets/widget-calendar.phpplayground/3-widgets/widget-confirm.phpplayground/3-widgets/widget-filepicker.phpplayground/3-widgets/widget-multifilepicker.phpplayground/3-widgets/widget-multisearch.phpplayground/3-widgets/widget-multiselect-groups.phpplayground/3-widgets/widget-multiselect.phpplayground/3-widgets/widget-number.phpplayground/3-widgets/widget-password-reveal.phpplayground/3-widgets/widget-password.phpplayground/3-widgets/widget-pause.phpplayground/3-widgets/widget-reorder.phpplayground/3-widgets/widget-search.phpplayground/3-widgets/widget-select-groups.phpplayground/3-widgets/widget-select.phpplayground/3-widgets/widget-suggest.phpplayground/3-widgets/widget-text.phpplayground/3-widgets/widget-textarea.phpplayground/3-widgets/widget-toggle.phpplayground/3-widgets/widgets.phpplayground/4-nested-panels/run.phpplayground/6-theme-detect/run.phpplayground/7-theme-options/run.phpplayground/8-key-bindings/run.phpplayground/9-bordered-panels/run.phpplayground/README.mdsrc/Answers/Answer.phpsrc/Answers/Answers.phpsrc/Answers/SummaryFormatter.phpsrc/Builder/FieldBuilder.phpsrc/Builder/Form.phpsrc/Builder/PanelBuilder.phpsrc/Config/ConfigException.phpsrc/Derive/Derive.phpsrc/Engine/Engine.phpsrc/Input/DefaultKeyMap.phpsrc/Input/KeyMap.phpsrc/Input/KeyMapManager.phpsrc/Input/Scope.phpsrc/Input/VimKeyMap.phpsrc/Model/DateBounds.phpsrc/Model/Field.phpsrc/Model/FieldType.phpsrc/Model/FilePickerMode.phpsrc/Model/Fixup.phpsrc/Model/FormDefinition.phpsrc/Model/FormException.phpsrc/Model/NumberBounds.phpsrc/Model/Option.phpsrc/Model/OptionKind.phpsrc/Model/Panel.phpsrc/Model/RenderMode.phpsrc/Model/Weekday.phpsrc/Render/Navigator.phpsrc/Render/PanelController.phpsrc/Resolver/InputResolver.phpsrc/Schema/AgentHelp.phpsrc/Schema/SchemaGenerator.phpsrc/Schema/SchemaValidator.phpsrc/Testing/TuiTester.phpsrc/Theme/DefaultTheme.phpsrc/Theme/ThemeManager.phpsrc/Tui.phpsrc/Widget/CalendarWidget.phpsrc/Widget/Capability/FilterCapableTrait.phpsrc/Widget/Capability/MultiSelectionCapableTrait.phpsrc/Widget/Capability/OptionsCapableInterface.phpsrc/Widget/Capability/OptionsCapableTrait.phpsrc/Widget/Capability/SearchCapableTrait.phpsrc/Widget/Capability/SelectionCapableTrait.phpsrc/Widget/ConfirmWidget.phpsrc/Widget/FilePickerWidget.phpsrc/Widget/Matcher.phpsrc/Widget/MultiSearchWidget.phpsrc/Widget/MultiSelectWidget.phpsrc/Widget/NumberWidget.phpsrc/Widget/PasswordWidget.phpsrc/Widget/PauseWidget.phpsrc/Widget/ReorderWidget.phpsrc/Widget/SearchWidget.phpsrc/Widget/SelectWidget.phpsrc/Widget/TextWidget.phpsrc/Widget/TextareaWidget.phpsrc/Widget/ToggleWidget.phpsrc/Widget/WidgetFactory.phptests/phpunit/Traits/MixedOptionsTrait.phptests/phpunit/Unit/Answers/AnswersTest.phptests/phpunit/Unit/Answers/SummaryFormatterTest.phptests/phpunit/Unit/Builder/FormTest.phptests/phpunit/Unit/Derive/DeriveTest.phptests/phpunit/Unit/Engine/EngineAnswersTest.phptests/phpunit/Unit/Engine/EngineConditionalTest.phptests/phpunit/Unit/Engine/EngineDeclaredBehaviourTest.phptests/phpunit/Unit/Engine/EngineDeriveTest.phptests/phpunit/Unit/Engine/EngineNonInteractiveTest.phptests/phpunit/Unit/Engine/EngineTest.phptests/phpunit/Unit/Input/KeyMapTest.phptests/phpunit/Unit/Model/BuiltModelTest.phptests/phpunit/Unit/Model/DateBoundsTest.phptests/phpunit/Unit/Model/FormDefinitionTest.phptests/phpunit/Unit/Model/NumberBoundsTest.phptests/phpunit/Unit/Model/OptionTest.phptests/phpunit/Unit/Model/WeekdayTest.phptests/phpunit/Unit/Render/NavigatorTest.phptests/phpunit/Unit/Render/PanelControllerTest.phptests/phpunit/Unit/Resolver/InputResolverTest.phptests/phpunit/Unit/Schema/AgentHelpTest.phptests/phpunit/Unit/Schema/SchemaGeneratorTest.phptests/phpunit/Unit/Schema/SchemaValidatorTest.phptests/phpunit/Unit/Testing/AllWidgetsFormTest.phptests/phpunit/Unit/Theme/ThemeOptionsTest.phptests/phpunit/Unit/Theme/ThemeRenderTest.phptests/phpunit/Unit/Translation/TranslationRenderTest.phptests/phpunit/Unit/TuiTest.phptests/phpunit/Unit/Widget/CalendarWidgetTest.phptests/phpunit/Unit/Widget/FilePickerWidgetTest.phptests/phpunit/Unit/Widget/MatcherTest.phptests/phpunit/Unit/Widget/NumberWidgetTest.phptests/phpunit/Unit/Widget/SelectWidgetTest.phptests/phpunit/Unit/Widget/WidgetFactoryTest.php
💤 Files with no reviewable changes (1)
- src/Config/ConfigException.php
| $form = new FormDefinition( | ||
| $this->title, | ||
| $this->subject, | ||
| $panels, | ||
| $this->fixups, | ||
| $this->theme, | ||
| $this->envPrefix, | ||
| $this->banner, | ||
| $this->buttons, | ||
| $this->submitLabel, | ||
| $this->cancelLabel, | ||
| $this->clearOnExit, | ||
| $this->color, | ||
| $this->unicode, | ||
| $this->envPrefix, | ||
| $this->themeOptions, | ||
| KeyMapManager::create($this->keymap, $this->keymapOverrides), | ||
| $this->footer, | ||
| $this->translator, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Complete the declared FormDefinition / Appearance split.
FormDefinition still receives banner and button presentation state, while the tester and architecture docs expose no Appearance contract. This leaves the PR’s advertised API split unimplemented.
src/Builder/Form.php#L187-L197: move banner/button presentation state out ofFormDefinitionintoAppearance.src/Testing/TuiTester.php#L89-L97: accept and forward an optionalAppearanceso tests can configure the facade contract.tests/phpunit/Unit/Model/BuiltModelTest.php#L93-L103: move chrome-default assertions toAppearancetests.docs/architecture/architecture.puml#L16-L17: addAppearanceand itsTuiownership/dependencies.docs/content/architecture.mdx#L30-L30,41-L43: describeAppearanceas the presentation/runtime model.
📍 Affects 5 files
src/Builder/Form.php#L187-L197(this comment)src/Testing/TuiTester.php#L89-L97tests/phpunit/Unit/Model/BuiltModelTest.php#L93-L103docs/architecture/architecture.puml#L16-L17docs/content/architecture.mdx#L30-L30
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Builder/Form.php` around lines 187 - 197, Complete the
FormDefinition/Appearance split: in src/Builder/Form.php lines 187-197, remove
banner and button presentation state from FormDefinition and provide it through
Appearance; in src/Testing/TuiTester.php lines 89-97, accept and forward an
optional Appearance. Move chrome-default assertions from
src/tests/phpunit/Unit/Model/BuiltModelTest.php lines 93-103 into
Appearance-focused tests, and update docs/architecture/architecture.puml lines
16-17 plus docs/content/architecture.mdx lines 30 and 41-43 to define Appearance
as the presentation/runtime model with its Tui ownership and dependencies.
Source: Coding guidelines
There was a problem hiding this comment.
This describes the earlier two-layer design; the PR has since moved to a three-layer split, so there is no Appearance model to complete.
The three layers are: the widget layer (Field), the form layer (FormDefinition, which owns the questionnaire and its own chrome - the start banner and the submit/cancel buttons), and the global TUI runtime. The runtime is configured with fluent setters on the Tui facade (theme(), keys(), color(), unicode(), footer(), clearOnExit(), translator()) rather than a separate model object.
Banner and buttons are intentionally form chrome, not terminal-runtime settings, so they belong on FormDefinition; TuiTester no longer takes an Appearance; and the architecture diagram/docs describe the facade-owned runtime rather than an Appearance model. So there is no split left to finish - this is the intended shape.
There was a problem hiding this comment.
@AlexSkrypnyk Thanks for the clarification — that makes sense. Withdrawing this comment since it was based on the earlier two-layer design described in the PR objectives, which has since evolved into the three-layer split you describe.
✏️ Learnings added
Learnt from: AlexSkrypnyk
Repo: drevops/tui PR: 56
File: src/Builder/Form.php:187-197
Timestamp: 2026-07-16T02:36:55.928Z
Learning: In the `drevops/tui` PHP repository, the architecture is a three-layer split, not a `FormDefinition`/`Appearance` two-model split: (1) the widget layer (`DrevOps\Tui\Model\Field`), (2) the form layer (`DrevOps\Tui\Model\FormDefinition`, which owns the questionnaire plus its own chrome — start banner via `banner` and submit/cancel buttons via `buttons`/`submitLabel`/`cancelLabel`), and (3) the global TUI runtime, which is configured via fluent setters directly on the `DrevOps\Tui\Tui` facade (`theme()`, `keys()`, `color()`, `unicode()`, `footer()`, `clearOnExit()`, `translator()`) rather than through a separate `Appearance` model class. Banner and buttons are intentionally form chrome (on `FormDefinition`), not terminal-runtime settings. There is no `Appearance` model to introduce; `TuiTester` does not accept one.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
…and renamed the 'Config' namespace to 'Model'.
00289dd to
63e6b4f
Compare
This comment has been minimized.
This comment has been minimized.
|
Summary
Splits the single
Configmodel - which conflated the questionnaire with global terminal settings - into three clearly separated configuration layers: the form (a specific questionnaire), the widget (an individual field), and the TUI runtime (global settings shared by every form an app runs). Settings that describe the terminal rather than the questionnaire -theme, key bindings,color/unicodeforcing, the key-hint footer, screen clearing and the active language - move off theFormbuilder and onto theTuifacade as fluent setters, so two forms in one app no longer each carry their own copy of the terminal's theme or keymap. TheDrevOps\Tui\Confignamespace is renamed toDrevOps\Tui\Model.Changes
DrevOps\Tui\Configrenamed toDrevOps\Tui\Model;Configmodel renamed toFormDefinition;ConfigExceptionrenamed toFormException.FormDefinition, built by theFormbuilder) - carries the questionnaire and its own chrome:title,subject,panels,fixups,envPrefix,bannerand the submit/cancelbuttonsand labels.Field) - unchanged; every per-field option already lived here.Tuifacade gains fluent setterstheme(),keys(),color(),unicode(),footer(),clearOnExit()andtranslator(), and holds those settings for the whole run.PanelControllernow receives the resolved theme, keymap, footer and clear-on-exit flags directly.Tui::config()becomesTui::form(); theFormbuilder loses its TUI setters. The common pathnew Tui($form)is unchanged.Before / After
Summary by CodeRabbit
New Features
Tui::form()for accessing the built form definition.Changes
Tui.Documentation