feat: add Run All Phases toggle to wizard and resize modal#507
Open
pedramamini wants to merge 3 commits intomainfrom
Open
feat: add Run All Phases toggle to wizard and resize modal#507pedramamini wants to merge 3 commits intomainfrom
pedramamini wants to merge 3 commits intomainfrom
Conversation
- Resize wizard modal from max-w-5xl (1024px) to 1200px wide / 85vh tall, matching Director's Notes dimensions - Add runAllDocuments toggle to PhaseReviewScreen (Step 5), visible when there are 2+ generated documents - When enabled, all documents with tasks are included in the batch config instead of only the first - Extract reusable ToggleSwitch component to ui/ and refactor SettingCheckbox to use it - Add test for run-all-documents batch behavior Closes #490
📝 WalkthroughWalkthroughAdds an "Auto-run all phases" option: a new Changes
Sequence DiagramsequenceDiagram
participant User
participant PhaseReviewScreen
participant WizardContext
participant useWizardHandlers
participant BatchProcessor
User->>PhaseReviewScreen: Toggle "Run All documents" ON
PhaseReviewScreen->>WizardContext: setRunAllDocuments(true)
WizardContext->>WizardContext: update state & persist
User->>PhaseReviewScreen: Click Launch Session
PhaseReviewScreen->>useWizardHandlers: handleWizardLaunchSession()
useWizardHandlers->>WizardContext: read runAllDocuments
alt runAllDocuments = true
useWizardHandlers->>useWizardHandlers: build batch with ALL documents
else runAllDocuments = false
useWizardHandlers->>useWizardHandlers: build batch with FIRST document only
end
useWizardHandlers->>BatchProcessor: startBatchRun(BatchRunConfig)
BatchProcessor->>BatchProcessor: process documents sequentially
Possibly related PRs
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
Greptile SummaryThis PR adds a "Run All Phases" toggle to the wizard's Step 5 review screen, resizes the modal to 1200px/85vh, and extracts a reusable Key changes:
All changes are straightforward and well-integrated. Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant PhaseReviewScreen
participant WizardContext
participant useWizardHandlers
participant startBatchRun
User->>PhaseReviewScreen: Toggles "Run All Phases" switch
PhaseReviewScreen->>WizardContext: setRunAllDocuments(true/false)
WizardContext->>WizardContext: dispatch SET_RUN_ALL_DOCUMENTS
WizardContext-->>PhaseReviewScreen: state.runAllDocuments updated
User->>PhaseReviewScreen: Clicks "I'm Ready to Go"
PhaseReviewScreen->>useWizardHandlers: handleWizardLaunchSession(false)
useWizardHandlers->>useWizardHandlers: filter generatedDocuments where taskCount > 0
alt runAllDocuments = true
useWizardHandlers->>useWizardHandlers: docsToRun = all docsWithTasks
else runAllDocuments = false
useWizardHandlers->>useWizardHandlers: docsToRun = [docsWithTasks[0]]
end
useWizardHandlers->>useWizardHandlers: build BatchRunConfig with docsToRun
useWizardHandlers->>startBatchRun: startBatchRun(sessionId, batchConfig, autoRunFolderPath)
startBatchRun-->>User: All selected documents queued for batch run
Last reviewed commit: af3ec11 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/SettingCheckbox.tsx`:
- Line 66: The ToggleSwitch instance in SettingCheckbox (rendering ToggleSwitch
checked={checked} onChange={onChange} theme={theme}) is missing an accessible
name; update the JSX to pass an accessible label prop (e.g., ariaLabel or
aria-label depending on ToggleSwitch's API) populated from the setting's visible
label or a descriptive string (for example use the component prop/variable that
holds the setting name or `label`) so screen readers can identify the control;
ensure you use the ToggleSwitch prop name it expects (ariaLabel vs aria-label)
and keep the text concise and descriptive.
In `@src/renderer/components/ui/ToggleSwitch.tsx`:
- Around line 29-44: The ToggleSwitch component's button element is missing an
explicit type, causing it to act as a submit button inside forms; update the
button in ToggleSwitch.tsx (the element that uses onClick and props checked,
disabled, onChange, ariaLabel) to include type="button" to prevent unintended
form submissions while preserving the existing onClick, aria attributes and
styles.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/__tests__/renderer/hooks/useWizardHandlers.test.tssrc/renderer/components/SettingCheckbox.tsxsrc/renderer/components/Wizard/MaestroWizard.tsxsrc/renderer/components/Wizard/WizardContext.tsxsrc/renderer/components/Wizard/screens/PhaseReviewScreen.tsxsrc/renderer/components/ui/ToggleSwitch.tsxsrc/renderer/components/ui/index.tssrc/renderer/hooks/wizard/useWizardHandlers.ts
- Add explicit type="button" to ToggleSwitch to prevent unintended form
submissions
- Pass ariaLabel={title} in SettingCheckbox for screen reader accessibility
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/renderer/components/ui/ToggleSwitch.tsx (1)
12-13: Make accessible naming non-optional in the component API.
ariaLabelis optional on Line 12, so new call sites can accidentally render an unnamed switch. Prefer requiring an accessible name at the prop level to prevent silent a11y regressions.Suggested API tightening
export interface ToggleSwitchProps { /** Whether the toggle is on */ checked: boolean; /** Callback when the toggle state changes */ onChange: (checked: boolean) => void; /** The current theme */ theme: Theme; - /** Optional aria-label for accessibility */ - ariaLabel?: string; + /** aria-label for accessibility */ + ariaLabel: string; /** Whether the toggle is disabled */ disabled?: boolean; }Also applies to: 43-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/ui/ToggleSwitch.tsx` around lines 12 - 13, The ToggleSwitch component currently declares ariaLabel as optional which can lead to unnamed controls; make the accessible name required by removing the optional marker from the prop (e.g., in the ToggleSwitchProps or the ToggleSwitch component signature where ariaLabel?: string is defined) and update any internal handling that assumed it might be undefined (and update call sites to pass a string). Also update any defaultProps or tests referencing ToggleSwitch to supply a non-empty ariaLabel so the component API enforces an accessible name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/ui/ToggleSwitch.tsx`:
- Around line 29-45: The ToggleSwitch button lacks explicit keyboard focus
hooks; add a tabIndex and focus handlers to make focus behavior explicit and
stylable: in the ToggleSwitch component, add tabIndex={disabled ? -1 : 0} to the
button, introduce local state (e.g., isFocused) and onFocus/onBlur handlers that
set/unset it, update the button className or inline style to reflect focus
(e.g., add a focus ring class when isFocused), and if the component accepts
optional onFocus/onBlur props forward/invoke them from these handlers; keep
existing behavior for checked, onChange, disabled and ariaLabel unchanged.
---
Nitpick comments:
In `@src/renderer/components/ui/ToggleSwitch.tsx`:
- Around line 12-13: The ToggleSwitch component currently declares ariaLabel as
optional which can lead to unnamed controls; make the accessible name required
by removing the optional marker from the prop (e.g., in the ToggleSwitchProps or
the ToggleSwitch component signature where ariaLabel?: string is defined) and
update any internal handling that assumed it might be undefined (and update call
sites to pass a string). Also update any defaultProps or tests referencing
ToggleSwitch to supply a non-empty ariaLabel so the component API enforces an
accessible name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4807739a-ea23-4980-b552-4ade4e447337
📒 Files selected for processing (2)
src/renderer/components/SettingCheckbox.tsxsrc/renderer/components/ui/ToggleSwitch.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/components/SettingCheckbox.tsx
Comment on lines
+29
to
+45
| <button | ||
| type="button" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| if (!disabled) onChange(!checked); | ||
| }} | ||
| className={`relative w-10 h-5 rounded-full transition-colors flex-shrink-0 ${ | ||
| disabled ? 'opacity-50 cursor-not-allowed' : 'cursor-pointer' | ||
| }`} | ||
| style={{ | ||
| backgroundColor: checked ? theme.colors.accent : theme.colors.bgActivity, | ||
| }} | ||
| role="switch" | ||
| aria-checked={checked} | ||
| aria-label={ariaLabel} | ||
| disabled={disabled} | ||
| > |
There was a problem hiding this comment.
Add explicit keyboard focus hooks required by renderer component guidelines.
On Line 29, this interactive control is focusable but currently lacks tabIndex and focus handlers. Please add tabIndex and onFocus/onBlur so focus behavior is explicit and consistently stylable.
Suggested update
export function ToggleSwitch({
checked,
onChange,
theme,
ariaLabel,
disabled = false,
}: ToggleSwitchProps): React.ReactElement {
+ const [isFocused, setIsFocused] = React.useState(false);
+
return (
<button
type="button"
+ tabIndex={disabled ? -1 : 0}
+ onFocus={() => setIsFocused(true)}
+ onBlur={() => setIsFocused(false)}
onClick={(e) => {
e.stopPropagation();
if (!disabled) onChange(!checked);
}}
className={`relative w-10 h-5 rounded-full transition-colors flex-shrink-0 ${
disabled ? 'opacity-50 cursor-not-allowed' : 'cursor-pointer'
- }`}
+ } ${isFocused ? 'ring-2 ring-offset-2' : ''}`}
style={{
backgroundColor: checked ? theme.colors.accent : theme.colors.bgActivity,
}}
role="switch"
aria-checked={checked}
aria-label={ariaLabel}
disabled={disabled}
>As per coding guidelines, src/renderer/components/**/*.{ts,tsx}: Add tabIndex attribute and focus event handlers when implementing components that need keyboard focus.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/ui/ToggleSwitch.tsx` around lines 29 - 45, The
ToggleSwitch button lacks explicit keyboard focus hooks; add a tabIndex and
focus handlers to make focus behavior explicit and stylable: in the ToggleSwitch
component, add tabIndex={disabled ? -1 : 0} to the button, introduce local state
(e.g., isFocused) and onFocus/onBlur handlers that set/unset it, update the
button className or inline style to reflect focus (e.g., add a focus ring class
when isFocused), and if the component accepts optional onFocus/onBlur props
forward/invoke them from these handlers; keep existing behavior for checked,
onChange, disabled and ariaLabel unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ToggleSwitchcomponent toui/and refactoredSettingCheckboxto use itTest plan
runAllDocumentsis trueCloses #490
Summary by CodeRabbit
New Features
UI/Style
Tests