Release v0.1.8 — Phase 8: Dynamic Settings Form#120
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…p-guard CONV-096: Add settings step guard
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ptions-form-partial CONV-097: Create dynamic options form partial
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…eld-renderer CONV-098: Add segmented field renderer
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-renderer CONV-099: Add select field renderer
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-renderer CONV-100: Add toggle field renderer
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…renderer CONV-101: Add color field renderer
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ange-field-renderer CONV-102: Add number and range field renderer
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ptions-schema-on-target-selection CONV-103: Load converter options schema on target selection
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lt-converter-options CONV-104: Initialize default converter options
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…g-settings CONV-105: Render PNG to JPG settings
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…f-settings CONV-106: Render PNG to PDF settings
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ld-validation-errors CONV-107: Add settings field validation errors
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…s-before-convert-step CONV-108: Validate settings before convert step
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…s-state-during-navigation CONV-109: Preserve settings state during navigation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…st-placeholder CONV-110: Add estimated cost placeholder
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ings-smoke-tests CONV-111: Add dynamic settings smoke tests
📝 WalkthroughWalkthroughThis PR adds a complete settings phase to the DashboardConverter Livewire component. Users select a target format, load converter-specific options from a schema, configure them via dynamically rendered field UI, and validate before conversion. Exception handling maps validation errors to field-level Livewire errors, and options are cached per target format for navigation persistence. ChangesSettings Phase Workflow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
tests/Feature/Livewire/DashboardConverterSettingsNavigationTest.php (2)
39-50:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSame method name issue as previous tests.
This test also calls
goToFormatStep()on line 47. Verify this matches the actual method name in the component.🤖 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 `@tests/Feature/Livewire/DashboardConverterSettingsNavigationTest.php` around lines 39 - 50, The test calls goToFormatStep() on the DashboardConverter Livewire component but the component uses a different method name; update the test to call the actual method on the component (e.g., rename the test call from goToFormatStep() to the real method name defined in the DashboardConverter component such as goToFormat, navigateToFormatStep, or whatever the component exposes), ensuring the Livewire::test chain uses the correct method name and that assertions still verify 'step' is 'format' and 'currentFileId' remains the file id.
22-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSame method name issue as previous test.
This test also calls
goToFormatStep()on line 31. Verify this matches the actual method name in the component.🤖 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 `@tests/Feature/Livewire/DashboardConverterSettingsNavigationTest.php` around lines 22 - 37, The test calls the Livewire method goToFormatStep() on the DashboardConverter component but the component uses a different method name; open the DashboardConverter Livewire component, confirm the actual method that advances to the format step (e.g., goToFormatStep vs goToFormat or goToConversionStep), and update the test in DashboardConverterSettingsNavigationTest.php to call the correct method name (replace call('goToFormatStep') with the component's actual method) so the test matches the component API.
🧹 Nitpick comments (1)
tests/Feature/Livewire/DashboardConverterSettingsFieldsTest.php (1)
132-162: ⚡ Quick winConsider splitting into separate tests for number and range fields.
This test combines two distinct field types (number and range) in a single test case. Splitting them would improve test isolation, making it easier to identify which specific field type fails and improving debugging clarity.
♻️ Proposed refactor to split into two tests
-it('renders number and range option fields with bounds', function () { +it('renders a number option field with bounds', function () { settingsComponent( schema: [ [ 'key' => 'width', 'type' => 'number', 'label' => 'Width', 'default' => 1200, 'min' => 1, 'max' => 10000, ], - [ - 'key' => 'compression', - 'type' => 'range', - 'label' => 'Compression', - 'default' => 80, - 'min' => 0, - 'max' => 100, - 'step' => 1, - ], ], - options: ['width' => 1200, 'compression' => 80], + options: ['width' => 1200], ) ->assertSee('Width') - ->assertSee('Compression') ->assertSeeHtml('type="number"') + ->assertSeeHtml('wire:model.live.debounce.300ms="options.width"'); +}); + +it('renders a range option field with bounds and step', function () { + settingsComponent( + schema: [ + [ + 'key' => 'compression', + 'type' => 'range', + 'label' => 'Compression', + 'default' => 80, + 'min' => 0, + 'max' => 100, + 'step' => 1, + ], + ], + options: ['compression' => 80], + ) + ->assertSee('Compression') ->assertSeeHtml('type="range"') - ->assertSeeHtml('wire:model.live.debounce.300ms="options.width"') ->assertSeeHtml('wire:model.live="options.compression"') ->assertSeeHtml('data-testid="range-value-compression"'); });🤖 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 `@tests/Feature/Livewire/DashboardConverterSettingsFieldsTest.php` around lines 132 - 162, The test it('renders number and range option fields with bounds', which uses settingsComponent with schema keys 'width' (type 'number') and 'compression' (type 'range'), should be split into two focused tests: one that only asserts the number field behavior for 'width' (presence of label, type="number", wire:model.live.debounce...="options.width", min/max/default) and a second that only asserts the range field behavior for 'compression' (label, type="range", wire:model.live="options.compression", step/min/max/default and presence of data-testid="range-value-compression"); create two test cases named clearly (e.g., it('renders number option field with bounds') and it('renders range option field with bounds')), each calling settingsComponent with a single-field schema and asserting only the relevant expectations to improve isolation and debuggability.
🤖 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
`@resources/views/livewire/dashboard/dashboard-converter/fields/toggle.blade.php`:
- Around line 16-18: The focus ring is applied to the non-focusable track span
(class "ca-focus-ring") instead of the checkbox input ("peer sr-only"); either
move the "ca-focus-ring" class from the track span to the input element (the
checkbox with classes "peer sr-only") so the actual focus-visible styles fire,
or keep the class on the span but change it to use the peer focus variant (e.g.,
replace "ca-focus-ring" on the span with a peer-based variant like
"peer-focus-visible:ca-focus-ring") so the visual track receives the focus ring
when the hidden input gains keyboard focus.
In `@tests/Feature/Livewire/DashboardConverterSettingsSmokeTest.php`:
- Around line 33-49: The test is missing a Livewire no-errors guard so component
errors can be masked; update the Livewire test chain for DashboardConverter (the
sequence that calls
selectTargetFormat('jpg')->call('goToFormatStep')->call('selectTargetFormat','pdf'))
to include ->assertHasNoErrors() after the interactions that may trigger
validation/state errors (e.g., after the first selectTargetFormat/goToFormatStep
sequence or immediately before asserting visibility) so Livewire failures
surface instead of leaving stale UI that makes assertSee/assertDontSee pass.
---
Duplicate comments:
In `@tests/Feature/Livewire/DashboardConverterSettingsNavigationTest.php`:
- Around line 39-50: The test calls goToFormatStep() on the DashboardConverter
Livewire component but the component uses a different method name; update the
test to call the actual method on the component (e.g., rename the test call from
goToFormatStep() to the real method name defined in the DashboardConverter
component such as goToFormat, navigateToFormatStep, or whatever the component
exposes), ensuring the Livewire::test chain uses the correct method name and
that assertions still verify 'step' is 'format' and 'currentFileId' remains the
file id.
- Around line 22-37: The test calls the Livewire method goToFormatStep() on the
DashboardConverter component but the component uses a different method name;
open the DashboardConverter Livewire component, confirm the actual method that
advances to the format step (e.g., goToFormatStep vs goToFormat or
goToConversionStep), and update the test in
DashboardConverterSettingsNavigationTest.php to call the correct method name
(replace call('goToFormatStep') with the component's actual method) so the test
matches the component API.
---
Nitpick comments:
In `@tests/Feature/Livewire/DashboardConverterSettingsFieldsTest.php`:
- Around line 132-162: The test it('renders number and range option fields with
bounds', which uses settingsComponent with schema keys 'width' (type 'number')
and 'compression' (type 'range'), should be split into two focused tests: one
that only asserts the number field behavior for 'width' (presence of label,
type="number", wire:model.live.debounce...="options.width", min/max/default) and
a second that only asserts the range field behavior for 'compression' (label,
type="range", wire:model.live="options.compression", step/min/max/default and
presence of data-testid="range-value-compression"); create two test cases named
clearly (e.g., it('renders number option field with bounds') and it('renders
range option field with bounds')), each calling settingsComponent with a
single-field schema and asserting only the relevant expectations to improve
isolation and debuggability.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: debd9a4a-96f0-4581-ac4b-f3bf4216b604
📒 Files selected for processing (15)
app/Livewire/Dashboard/DashboardConverter.phpapp/Support/Converters/Exceptions/InvalidConverterOptionsException.phpresources/views/livewire/dashboard/dashboard-converter.blade.phpresources/views/livewire/dashboard/dashboard-converter/fields/color.blade.phpresources/views/livewire/dashboard/dashboard-converter/fields/number.blade.phpresources/views/livewire/dashboard/dashboard-converter/fields/range.blade.phpresources/views/livewire/dashboard/dashboard-converter/fields/segmented.blade.phpresources/views/livewire/dashboard/dashboard-converter/fields/select.blade.phpresources/views/livewire/dashboard/dashboard-converter/fields/toggle.blade.phpresources/views/livewire/dashboard/dashboard-converter/partials/dynamic-options-form.blade.phptests/Feature/Livewire/DashboardConverterSettingsFieldsTest.phptests/Feature/Livewire/DashboardConverterSettingsNavigationTest.phptests/Feature/Livewire/DashboardConverterSettingsSmokeTest.phptests/Feature/Livewire/DashboardConverterSettingsStepTest.phptests/Feature/Livewire/DashboardConverterSettingsValidationTest.php
Address Phase 8 review: - Toggle focus ring was on the non-focusable track span; ca-focus-ring keys on the element's own :focus-visible, so it never fired for the hidden peer input. Add a .peer:focus-visible ~ .ca-focus-ring-peer rule and apply it to the track. - Add assertHasNoErrors() to the PNG->PDF smoke test so Livewire failures surface instead of leaving stale UI that lets assertSee/assertDontSee pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fix toggle focus ring and harden settings smoke test
Release v0.1.8 — Phase 8: Dynamic Settings Form
Adds the third step of the main flow — a fully schema-driven settings form — between target selection and a convert placeholder.
Scope (CONV-096 → CONV-111)
optionsSchema()on target selectionOptionsValidator(InvalidConverterOptionsExceptionnow carries the option key)Architecture
The UI never hardcodes PNG→JPG / PNG→PDF forms.
DashboardConverterloads the selected converter's schema;dynamic-options-formrenders fields purely from that schema and delegates to per-type renderers. The real converter schema is a list of fields (each with akey), so the form iterates the list.Out of scope (unchanged)
No ConversionJob, queue, real conversion, download, credits/billing, cost estimator, API, or preset persistence.
Verification
composer test— 233 passed (630 assertions)composer lint— passnpm run build— passphp artisan migrate:fresh --seed— pass🤖 Generated with Claude Code
Summary by cubic
Adds a schema-driven Settings step that renders converter options dynamically and validates them before continuing to the Convert placeholder. Enables distinct PNG→JPG and PNG→PDF configurations without hardcoded UI. (CONV-096 → CONV-111)
New Features
optionsSchema()with a visible fallback; fields: segmented, select, toggle, color, number, range.OptionsValidator;InvalidConverterOptionsExceptionincludes the option key; block continue and show inline errors.Bug Fixes
.peer:focus-visible ~ .ca-focus-ring-peer.assertHasNoErrors()to surface Livewire issues.Written for commit b519b13. Summary will update on new commits.
Summary by CodeRabbit