Conversation
enilsen16
force-pushed
the
pr/recipe-modal-cancel-fix
branch
from
May 12, 2026 19:43
6e7ba49 to
46ed722
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46ed722bc4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
enilsen16
force-pushed
the
pr/recipe-modal-cancel-fix
branch
from
May 12, 2026 20:40
46ed722 to
545c800
Compare
enilsen16
added a commit
to enilsen16/goose
that referenced
this pull request
May 12, 2026
ChatSessionsContainer keeps inactive sessions mounted, so two open
recipe sessions can each render a ParameterInputModal at the same time.
Using `id={param.key}` produced duplicate DOM IDs whenever two recipes
shared a parameter key, which means `<label htmlFor>` would resolve to
the first matching (often hidden) control — label clicks and assistive
tech ended up targeting the wrong field.
Generate a per-instance prefix with React's `useId` and route both the
label `htmlFor` and the input `id` through a small `fieldId` helper.
The DOM/label association is preserved, but two mounted modals no longer
collide.
Addresses Codex P2 on aaif-goose#9167 (ParameterInputModal.tsx:167).
Signed-off-by: Erik Nilsen <enilsen16@live.com>
…w instead of closing modal - Removed window.electron.createChatWindow() call when canceling recipe setup - When user selects 'Start New Chat (No Recipe)', modal now just closes (calls onClose()) - Previously this was creating an extra window instead of closing the modal - Removed unused getInitialWorkingDir import - Added comprehensive tests for ParameterInputModal component - Tests verify the bug fix and cover form submission, validation, and cancel behavior Signed-off-by: Erik Nilsen <enilsen16@live.com>
ChatSessionsContainer keeps inactive sessions mounted, so two open
recipe sessions can each render a ParameterInputModal at the same time.
Using `id={param.key}` produced duplicate DOM IDs whenever two recipes
shared a parameter key, which means `<label htmlFor>` would resolve to
the first matching (often hidden) control — label clicks and assistive
tech ended up targeting the wrong field.
Generate a per-instance prefix with React's `useId` and route both the
label `htmlFor` and the input `id` through a small `fieldId` helper.
The DOM/label association is preserved, but two mounted modals no longer
collide.
Addresses Codex P2 on aaif-goose#9167 (ParameterInputModal.tsx:167).
Signed-off-by: Erik Nilsen <enilsen16@live.com>
enilsen16
force-pushed
the
pr/recipe-modal-cancel-fix
branch
from
May 12, 2026 20:43
545c800 to
b465c9a
Compare
enilsen16
changed the title
Contributor
Author
|
Closing to reduce review queue. Can be reopened when ready for review. |
3 tasks
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
Fixes #8864. In the goose1 desktop app, hitting Cancel on the recipe-parameter form was spawning a fresh window instead of closing the modal — because the cancel path went through the same "launch with recipe" code as Submit.
Testing
ui/desktop/src/components/__tests__/ParameterInputModal.test.tsxcover Cancel and Submit paths.Related Issues
Fixes #8864.