Wizard: Work around FirstBoot editor losing keyboard focus (HMS-9956)#4003
Wizard: Work around FirstBoot editor losing keyboard focus (HMS-9956)#4003ochosi wants to merge 1 commit intoosbuild:mainfrom
Conversation
When typing the first character in the FirstBoot script editor, the Monaco editor would lose focus and cursor position, making typing impossible. Root cause: PatternFly's CodeEditor uses conditional JSX rendering based on `isUploadEnabled` and whether `code` is empty. When code transitions from empty to non-empty, React unmounts and remounts the Monaco editor component, causing focus and cursor state to be lost. The fix removes `isUploadEnabled` to prevent the JSX branch switch. This disables the "Browse" upload button but preserves copy, download, and "Start from scratch" functionality.
ochosi
requested review from
kingsleyzissou,
regexowl and
tkoscieln
and removed request for
a team
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Since this is a temporary workaround for an upstream bug, consider adding an inline comment next to the
CodeEditorprops (with a link to the upstream PR) explaining whyisUploadEnabledandemptyStateButtonare intentionally omitted so future maintainers know when it’s safe to revert. - If this focus issue may appear in other CodeEditor usages, consider wrapping CodeEditor in a small local wrapper component that encapsulates this workaround, so the behavior is consistent and the hack is centralized rather than being applied ad hoc in individual steps.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since this is a temporary workaround for an upstream bug, consider adding an inline comment next to the `CodeEditor` props (with a link to the upstream PR) explaining why `isUploadEnabled` and `emptyStateButton` are intentionally omitted so future maintainers know when it’s safe to revert.
- If this focus issue may appear in other CodeEditor usages, consider wrapping CodeEditor in a small local wrapper component that encapsulates this workaround, so the behavior is consistent and the hack is centralized rather than being applied ad hoc in individual steps.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
📝 WalkthroughWalkthroughThe FirstBoot component in the CreateImageWizard updates the CodeEditor configuration. The 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 |
ochosi
changed the title
|
My upstream PatternFly fix got merged, so let's close this and pull in the latest version of patternfly-react as soon as it's released. |
1 similar comment
|
My upstream PatternFly fix got merged, so let's close this and pull in the latest version of patternfly-react as soon as it's released. |
1 participant
Intro
This is a possible workaround for an upstream bug in patternfly-react/CodeEditor. An upstream fix has been submitted (patternfly/patternfly-react#12212). In case we consider the bug too grave, we can consider this minimal patch as a workaround.
The downsides are that the EmptyState is broken by this workaround and not shown at all. It would be possible to alleviate that, but I've tried it and it results in rewriting portions of CodeEditor inside image-builder-frontend, so I would advise against it.
Bug
When typing the first character in the FirstBoot script editor, the Monaco editor would lose focus and cursor position, making typing impossible.
Root cause
PatternFly's CodeEditor uses conditional JSX rendering based on
isUploadEnabledand whethercodeis empty. When code transitions from empty to non-empty, React unmounts and remounts the Monaco editor component, causing focus and cursor state to be lost.The fix removes
isUploadEnabledto prevent the JSX branch switch. This disables the "Browse" upload button but preserves copy, download, and "Start from scratch" functionality.