CNV-40684: Type into console with keymap #2486
Conversation
|
@rszwajko: This pull request references CNV-40684 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.19.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Skipping CI for Draft Pull Request. |
rszwajko
requested review from
adamviktora,
avivtur,
pcbailey and
upalatucci
rszwajko
force-pushed
the
keymapPaste
branch
from
95e644a to
a1c84cf
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rszwajko The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@rszwajko: This pull request references CNV-40684 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.19.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@rszwajko: This pull request references CNV-40684 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.19.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Create project utilities: 1. GitHub actions - based on GH provide templates 2. simple build based on tsup 3. test via jest 4. lintings with eslint Reference-Url: kubevirt-ui/kubevirt-plugin#2486 Signed-off-by: Radoslaw Szwajkowski <[email protected]>
Create project utilities: 1. GitHub actions - based on GH provide templates 2. simple build based on tsup 3. test via jest 4. lintings with eslint Reference-Url: kubevirt-ui/kubevirt-plugin#2486 Reference-Url: https://github.com/qemu/qemu/blob/b69801dd6b1eb4d107f7c2f643adf0a4e3ec9124/ui/vnc_keysym.h Reference-Url: https://github.com/qemu/qemu/tree/b69801dd6b1eb4d107f7c2f643adf0a4e3ec9124/pc-bios/keymaps Reference-Url: https://www.x.org/releases/X11R7.7/src/xserver/xorg-server-1.12.2.tar.gz Reference-Url: https://danielhb.github.io/article/2019/05/06/noVNC-QEMU-RFB.html Signed-off-by: Radoslaw Szwajkowski <[email protected]>
Create project utilities: 1. GitHub actions - based on GH provide templates 2. simple build based on tsup 3. test via jest 4. lintings with eslint Reference-Url: kubevirt-ui/kubevirt-plugin#2486 Reference-Url: https://github.com/qemu/qemu/blob/b69801dd6b1eb4d107f7c2f643adf0a4e3ec9124/ui/vnc_keysym.h Reference-Url: https://github.com/qemu/qemu/tree/b69801dd6b1eb4d107f7c2f643adf0a4e3ec9124/pc-bios/keymaps Reference-Url: https://www.x.org/releases/X11R7.7/src/xserver/xorg-server-1.12.2.tar.gz Reference-Url: https://danielhb.github.io/article/2019/05/06/noVNC-QEMU-RFB.html Signed-off-by: Radoslaw Szwajkowski <[email protected]>
Create project utilities: 1. GitHub actions - based on GH provide templates 2. dual publishing ESM and CJS build based on tsup 3. test via jest 4. lintings with eslint Reference-Url: kubevirt-ui/kubevirt-plugin#2486 Reference-Url: https://github.com/qemu/qemu/blob/b69801dd6b1eb4d107f7c2f643adf0a4e3ec9124/ui/vnc_keysym.h Reference-Url: https://github.com/qemu/qemu/tree/b69801dd6b1eb4d107f7c2f643adf0a4e3ec9124/pc-bios/keymaps Reference-Url: https://www.x.org/releases/X11R7.7/src/xserver/xorg-server-1.12.2.tar.gz Reference-Url: https://danielhb.github.io/article/2019/05/06/noVNC-QEMU-RFB.html Signed-off-by: Radoslaw Szwajkowski <[email protected]>
|
|
||
| import UnsupportedCharModal from './UnsupportedCharModal'; | ||
|
|
||
| const canExecuteCommnands = (rfb) => rfb._rfbConnectionState === 'connected' && !rfb._viewOnly; |
There was a problem hiding this comment.
We can use ConsoleState.connected
There was a problem hiding this comment.
We cannot because we are checking against internal RFB state - the names are the same but using our enum would suggest it's the same state.
src/utils/components/Consoles/components/vnc-console/actions.tsx
Outdated
Show resolved
Hide resolved
src/utils/components/Consoles/components/AccessConsoles/AccessConsoles.tsx
Show resolved
Hide resolved
rszwajko
force-pushed
the
keymapPaste
branch
from
dc83e59 to
9f941e2
Compare
|
@rszwajko: This pull request references CNV-40684 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.21.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
rszwajko
force-pushed
the
keymapPaste
branch
from
cd46610 to
76b815a
Compare
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR introduces VNC keyboard layout selection with favorite keymaps persistence, replaces static paste functionality with keymap-aware clipboard handling, adds modal-based validation for unsupported characters, updates CI/workflow to Node.js 22 and GitHub npm registry authentication, and adds new translation keys for clipboard/keyboard error messaging. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant VncKeymapDropdown as VncKeymapDropdown
participant AccessConsoles as AccessConsoles
participant sendPasteCMD as sendPasteCMD
participant Clipboard as Clipboard API
participant KeyMap as KeyMap Lookup
participant UnsupportedCharModal as UnsupportedCharModal
participant VNC as VNC Console
User->>VncKeymapDropdown: Open dropdown / Select keymap
VncKeymapDropdown->>VncKeymapDropdown: Update selectedKeyboard
VncKeymapDropdown->>VncKeymapDropdown: Manage favorites (localStorage)
User->>VncKeymapDropdown: Click Paste action
VncKeymapDropdown->>sendPasteCMD: Call with PasteParams<br/>(selectedKeyboard, createModal)
sendPasteCMD->>Clipboard: Read clipboard text
alt Valid String
Clipboard-->>sendPasteCMD: clipboard text
sendPasteCMD->>KeyMap: Map each char using<br/>selectedKeyboard layout
alt Unsupported chars found
rect rgb(255, 200, 200)
sendPasteCMD->>UnsupportedCharModal: Trigger modal<br/>via createModal()
User->>UnsupportedCharModal: View unsupported chars
User->>UnsupportedCharModal: Close
end
else All chars supported
rect rgb(200, 255, 200)
sendPasteCMD->>VNC: Send mapped keys<br/>with delays
VNC-->>sendPasteCMD: Keys emitted
end
end
else Invalid/Non-string
Clipboard-->>sendPasteCMD: null/non-string
sendPasteCMD->>sendPasteCMD: Abort
end
sendPasteCMD->>VNC: Focus (if requested)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
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 |
rszwajko
force-pushed
the
keymapPaste
branch
from
0e7d8eb to
5411a30
Compare
rszwajko
force-pushed
the
keymapPaste
branch
5 times, most recently
from
4befb88 to
66e05ba
Compare
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
@rszwajko: This pull request references CNV-40684 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.21.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
.npmrc (1)
1-2: Scoped registry looks good; ensure local dev guidance and consider setup-node auth.
- Keep this, but document how contributors set GITHUB_READ_PACKAGES_TOKEN locally (PAT with read:packages) or installs will 401.
- Optional: rely on actions/setup-node to inject auth and remove step‑level env duplication by setting with:
registry-url: https://npm.pkg.github.com and scope: '@kubevirt-ui'.locales/en/plugin__kubevirt-plugin.json (1)
1636-1636: Minor copy nit (optional).“Unsupported content in the clipboard” → “Unsupported clipboard content” (shorter, title-friendly). Optional.
- "Unsupported content in the clipboard": "Unsupported content in the clipboard", + "Unsupported clipboard content": "Unsupported clipboard content",src/utils/components/Consoles/components/vnc-console/actions.tsx (1)
18-129: VNC paste flow looks sound; consider small robustness & perf tweaksThe overall approach (connection checks, keymap lookup, unsupported-char modal, and QEMU extended key events with delays) looks solid. A few non-blocking improvements to consider:
- Reuse
canExecuteCommandsincreateSendKeyFunctionfor a single source of truth on connection/view-only checks.- Add a defensive guard for an unexpected/missing
keyMaps[selectedKeyboard](e.g. fallback to a default layout or early-return with a console error) to avoid a hard runtime error if an invalid layout ever slips through.- The fixed 50ms sleeps per key event can make large pastes quite slow (hundreds of ms per character). If testing shows it’s safe, you might reduce the delay or make it configurable to better balance reliability vs. throughput.
These are optional cleanups; the current implementation is functionally correct given the surrounding usage.
src/utils/components/Consoles/components/AccessConsoles/VncKeymapDropdown.tsx (2)
40-43: Validatevaluetype before callingupdateFavorite.The
onActionClickcallback receivesvaluetyped asnumber | string(from PatternFly's API), butupdateFavoriteexpects aKeyboardLayout. Consider adding type validation similar to theonSelecthandler.Apply this diff:
onActionClick={(event, value) => { event.stopPropagation(); - updateFavorite(value); + isKeyboardLayout(value) && updateFavorite(value); }}
92-99: Consider defensive null check forkeyMapsaccess.While
favoriteKeymapsis pre-filtered inuseFavoriteKeymapsto contain only known keymaps, adding a defensive check here would improve robustness against edge cases (e.g., stale localStorage data combined with library version mismatches).<DropdownGroup> {favoriteKeymaps .map((value): [KeyboardLayout, KeyMapDef] => [value, keyMaps[value]]) + .filter(([, def]) => def !== undefined) .map(([value, def]) => ( <DropdownItem description={value} isFavorited key={value} value={value}> {def.description} </DropdownItem> ))} </DropdownGroup>src/utils/components/Consoles/components/AccessConsoles/utils/accessConsoles.tsx (1)
63-65: Strengthen type validation for localStorage data.The array elements from
savedFavoriteKeymapsaren't explicitly validated as strings before filtering. While theknownKeymaps.has(entry)check will fail gracefully for non-string types, adding explicit type validation would improve type safety.const filteredFavoriteKeymaps: KeyboardLayout[] = ( Array.isArray(savedFavoriteKeymaps) ? savedFavoriteKeymaps : [] - ).filter((entry) => knownKeymaps.has(entry)); + ).filter((entry): entry is KeyboardLayout => + typeof entry === 'string' && knownKeymaps.has(entry) + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (18)
.github/workflows/ci_checks.yml(4 hunks).npmrc(1 hunks)locales/en/plugin__kubevirt-plugin.json(2 hunks)locales/es/plugin__kubevirt-plugin.json(2 hunks)locales/fr/plugin__kubevirt-plugin.json(2 hunks)locales/ja/plugin__kubevirt-plugin.json(2 hunks)locales/ko/plugin__kubevirt-plugin.json(2 hunks)locales/zh/plugin__kubevirt-plugin.json(2 hunks)package.json(1 hunks)src/utils/components/Consoles/ConsoleStandAlone.tsx(3 hunks)src/utils/components/Consoles/components/AccessConsoles/AccessConsoles.tsx(4 hunks)src/utils/components/Consoles/components/AccessConsoles/VncKeymapDropdown.tsx(1 hunks)src/utils/components/Consoles/components/AccessConsoles/utils/accessConsoles.tsx(3 hunks)src/utils/components/Consoles/components/SerialConsole/SerialConsole.tsx(2 hunks)src/utils/components/Consoles/components/vnc-console/UnsupportedCharModal.tsx(1 hunks)src/utils/components/Consoles/components/vnc-console/VncConsole.tsx(1 hunks)src/utils/components/Consoles/components/vnc-console/actions.ts(0 hunks)src/utils/components/Consoles/components/vnc-console/actions.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- src/utils/components/Consoles/components/vnc-console/actions.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-06T13:11:11.791Z
Learnt from: rszwajko
Repo: kubevirt-ui/kubevirt-plugin PR: 3089
File: locales/es/plugin__kubevirt-plugin.json:102-102
Timestamp: 2025-11-06T13:11:11.791Z
Learning: For the kubevirt-ui/kubevirt-plugin repository: Do not flag or comment on missing translations (English values) in locale files for languages other than English (e.g., locales/es/, locales/fr/, locales/ja/, locales/ko/, locales/zh/). Only review English locale files (locales/en/) for translation keys.
Applied to files:
locales/es/plugin__kubevirt-plugin.jsonlocales/zh/plugin__kubevirt-plugin.jsonlocales/ja/plugin__kubevirt-plugin.jsonlocales/en/plugin__kubevirt-plugin.jsonlocales/fr/plugin__kubevirt-plugin.jsonlocales/ko/plugin__kubevirt-plugin.json
🔇 Additional comments (10)
.github/workflows/ci_checks.yml (1)
25-25: Node 22 bump is toolchain-compatible — verified.Verification confirms compatibility: engines.node constraint
>=18.20.5allows Node 22, no native build dependencies present, and standard yarn lockfile v1 has no known issues. The bump is safe to proceed.locales/ko/plugin__kubevirt-plugin.json (1)
60-60: Skipping non-English locale review per repo policy.Per repository guidance, we only review English locale files; non‑English entries are not evaluated for translation quality. Based on learnings.
Also applies to: 1636-1636
locales/es/plugin__kubevirt-plugin.json (1)
65-65: Non-English locale diff acknowledged (no translation review)Per repo guidance, I’m not reviewing or flagging translation content in non-English locale files; the new keys structurally align with the rest of the file.
Also applies to: 644-644
locales/zh/plugin__kubevirt-plugin.json (1)
60-60: Non-English locale diff acknowledged (no translation review)Following project guidance, I’m not reviewing translation quality/content in non-English locale files; the newly added keys are structurally consistent.
Also applies to: 1636-1636
src/utils/components/Consoles/ConsoleStandAlone.tsx (1)
11-21: ModalProvider integration around stand-alone consoles looks correctWrapping
<Consoles>in<ModalProvider value={value}>withvaluefromuseModalValue()is a clean way to enable the new modal flow (e.g., unsupported-char modal) in the stand-alone console; hooks are used correctly and existing behavior is preserved.Also applies to: 34-46
package.json (1)
2-4: Dependency properly integrated; registry availability requires manual CI verificationThe
@kubevirt-ui/[email protected]package is correctly imported and used across four console components (vnc-console/actions.tsx, VncKeymapDropdown.tsx, AccessConsoles.tsx, and accessConsoles.tsx utilities), matching the keyboard layout mapping logic as described.However, registry availability cannot be verified in this environment since the package is published to GitHub Packages (which requires authentication). Ensure your CI/build environment has proper GitHub token configuration (via
GITHUB_TOKENenv var or.npmrccredentials) to resolve this private scope dependency during builds.src/utils/components/Consoles/components/AccessConsoles/AccessConsoles.tsx (1)
4-5: Verified: sendPaste implementations are consistently async with proper Promise handlingThe type definition in
accessConsoles.tsx:34explicitly requiressendPaste?: (params?: PasteParams) => Promise<void>. Both implementations—SerialConsole's inline async function (line 117) and VNC'ssendPasteCMD(vnc-console/actions.tsx:40)—are async and thus return Promise. Both call sites (VncKeymapDropdown.tsx:57 and AccessConsoles.tsx:109) correctly chain.catch()for error handling. Your concern has already been addressed by the type system and implementation.src/utils/components/Consoles/components/vnc-console/VncConsole.tsx (1)
41-49: Change is safe – no breaking named imports remainVerification confirms
VncConsoleis exported only as default (line 151:export default memo(VncConsole)), and no named imports ofVncConsoleexist anywhere in the codebase. The removal of the named export poses no breaking change risk.src/utils/components/Consoles/components/SerialConsole/SerialConsole.tsx (1)
117-126: LGTM! Clean API migration.The update to accept
PasteParamsinstead of a boolean maintains backward-compatible default behavior (shouldFocusOnConsole = true). While SerialConsole doesn't use all fields inPasteParams(e.g.,createModal,selectedKeyboard), this unified API design is appropriate for consistency across console types.src/utils/components/Consoles/components/AccessConsoles/utils/accessConsoles.tsx (1)
34-34: AllsendPastecall sites have been successfully migrated to the new signature.Verification across the codebase confirms that all invocations of
sendPasteare using the new object parameter pattern ({ shouldFocusOnConsole: true }or similar), with no remaining calls using the old boolean argument style. The migration is complete.
| permissions: | ||
| packages: read |
There was a problem hiding this comment.
build and unit-test lack packages: read; installs from GH Packages will fail.
Grant packages: read to all jobs (or at workflow level). Example (preferred, top-level):
name: CI
+permissions:
+ packages: read
on:Or per job:
i18n:
name: i18n
permissions:
packages: read
+
+build:
+ permissions:
+ packages: read
+
+unit-test:
+ permissions:
+ packages: readCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
.github/workflows/ci_checks.yml lines 15-16: the workflow only sets permissions:
packages: read for a single scope which is missing at job level so installs from
GitHub Packages will fail for build and unit-test jobs; update the workflow to
grant packages: read at the workflow-level permissions block (or add packages:
read to each job) so GH Packages pulls succeed, ensuring the permissions block
is at the top-level of the YAML and not nested under a specific job.
| "{{time}} seconds": "{{time}} seconds", | ||
| "{{timestampPluralized}} ago": "{{timestampPluralized}} ago", | ||
| "{{tolerations}} Toleration rules": "{{tolerations}} Toleration rules", | ||
| "{{unsupportedCharCount}} characters are not supported by the keyboard layout mapping:": "{{unsupportedCharCount}} characters are not supported by the keyboard layout mapping:", |
There was a problem hiding this comment.
Clarify wording; consider pluralization.
“keyboard layout mapping” is a bit clunky for users. Prefer concise phrasing and (optionally) plural handling.
Apply this small copy tweak:
- "{{unsupportedCharCount}} characters are not supported by the keyboard layout mapping:": "{{unsupportedCharCount}} characters are not supported by the keyboard layout mapping:",
+ "{{unsupportedCharCount}} characters are not supported by the selected keyboard layout:": "{{unsupportedCharCount}} characters are not supported by the selected keyboard layout:",Optional (if your i18n setup supports plural forms like _one/_other used elsewhere in this file), you could split for proper grammar:
+ "{{unsupportedCharCount}} characters are not supported by the selected keyboard layout:_one": "{{unsupportedCharCount}} character is not supported by the selected keyboard layout:",
+ "{{unsupportedCharCount}} characters are not supported by the selected keyboard layout:_other": "{{unsupportedCharCount}} characters are not supported by the selected keyboard layout:",And call with t(key, { count: unsupportedCharCount }).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "{{unsupportedCharCount}} characters are not supported by the keyboard layout mapping:": "{{unsupportedCharCount}} characters are not supported by the keyboard layout mapping:", | |
| "{{unsupportedCharCount}} characters are not supported by the selected keyboard layout:": "{{unsupportedCharCount}} characters are not supported by the selected keyboard layout:", |
🤖 Prompt for AI Agents
locales/en/plugin__kubevirt-plugin.json around line 60: the string uses clunky
phrasing and doesn't handle pluralization; change the key/value to clearer
wording (e.g., "Unsupported characters for keyboard layout:" or "Characters not
supported by the keyboard layout:") and if your i18n supports plural forms, add
two entries like key_one and key_other (or the _one/_other pattern used in this
file) where the _one form uses singular wording and _other uses plural, then
ensure callers invoke t(key, { count: unsupportedCharCount }) so the correct
form is used.
| const UnsupportedCharModal: FC<UnsupportedCharModal> = ({ isOpen, onClose, unsupportedChars }) => { | ||
| const { t } = useKubevirtTranslation(); | ||
| const maxChars = 10; | ||
| const unsupportedCharCount = unsupportedChars.length; | ||
| return ( | ||
| <Modal isOpen={isOpen} onClose={onClose} position="top" variant={'small'}> | ||
| <ModalHeader title={t('Unsupported content in the clipboard')} /> | ||
| <ModalBody> | ||
| {t( | ||
| '{{unsupportedCharCount}} characters are not supported by the keyboard layout mapping:', | ||
| { | ||
| unsupportedCharCount, | ||
| }, | ||
| )} | ||
| <List> | ||
| {unsupportedChars.splice(0, maxChars).map((char) => ( | ||
| <ListItem key={char}>{`'${char}' (0x${char.codePointAt(0).toString(16)})`}</ListItem> | ||
| ))} |
There was a problem hiding this comment.
Avoid mutating props when truncating the unsupported chars list
unsupportedChars.splice(0, maxChars) mutates the unsupportedChars prop during render, which is a React anti‑pattern and can lead to surprising behavior if the same array is reused.
Use a non‑mutating slice instead:
- <List>
- {unsupportedChars.splice(0, maxChars).map((char) => (
+ <List>
+ {unsupportedChars.slice(0, maxChars).map((char) => (
<ListItem key={char}>{`'${char}' (0x${char.codePointAt(0).toString(16)})`}</ListItem>
))}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const UnsupportedCharModal: FC<UnsupportedCharModal> = ({ isOpen, onClose, unsupportedChars }) => { | |
| const { t } = useKubevirtTranslation(); | |
| const maxChars = 10; | |
| const unsupportedCharCount = unsupportedChars.length; | |
| return ( | |
| <Modal isOpen={isOpen} onClose={onClose} position="top" variant={'small'}> | |
| <ModalHeader title={t('Unsupported content in the clipboard')} /> | |
| <ModalBody> | |
| {t( | |
| '{{unsupportedCharCount}} characters are not supported by the keyboard layout mapping:', | |
| { | |
| unsupportedCharCount, | |
| }, | |
| )} | |
| <List> | |
| {unsupportedChars.splice(0, maxChars).map((char) => ( | |
| <ListItem key={char}>{`'${char}' (0x${char.codePointAt(0).toString(16)})`}</ListItem> | |
| ))} | |
| const UnsupportedCharModal: FC<UnsupportedCharModal> = ({ isOpen, onClose, unsupportedChars }) => { | |
| const { t } = useKubevirtTranslation(); | |
| const maxChars = 10; | |
| const unsupportedCharCount = unsupportedChars.length; | |
| return ( | |
| <Modal isOpen={isOpen} onClose={onClose} position="top" variant={'small'}> | |
| <ModalHeader title={t('Unsupported content in the clipboard')} /> | |
| <ModalBody> | |
| {t( | |
| '{{unsupportedCharCount}} characters are not supported by the keyboard layout mapping:', | |
| { | |
| unsupportedCharCount, | |
| }, | |
| )} | |
| <List> | |
| {unsupportedChars.slice(0, maxChars).map((char) => ( | |
| <ListItem key={char}>{`'${char}' (0x${char.codePointAt(0).toString(16)})`}</ListItem> | |
| ))} |
🤖 Prompt for AI Agents
In src/utils/components/Consoles/components/vnc-console/UnsupportedCharModal.tsx
around lines 20 to 37, the render calls unsupportedChars.splice(0, maxChars)
which mutates the prop array; replace that with a non‑mutating slice (e.g.,
const displayed = unsupportedChars.slice(0, maxChars)) and map over displayed so
the original unsupportedChars is not modified during render.
0e48122 to
5bf7b36
Compare
|
@rszwajko: This pull request references CNV-40684 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.21.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Usage in the CI: 1. GITHUB_TOKEN with permissions reduced to packages:read 2. create .npmrc file with actions/setup-node 3. inject the token as NODE_AUTH_TOKEN env var Local development: The packages under @kubevirt-ui will be public but GitHub Packages requires authentication with personal access token to install. The minimum required scope is packages:read. One way to access the token is to use env var and amend the global .npmrc with the following 2 lines: @kubevirt-ui:registry=https://npm.pkg.github.com //npm.pkg.github.com/:_authToken=${GITHUB_READ_PACKAGES_TOKEN} Updates: 1. the Node version from v18 to v22 (the currently active LTS version) 2. actions/setup-node from v4 to v6 3. unify actions/checkout to v5 Reference-Url: https://docs.github.com/en/packages/learn-github-packages/about-permissions-for-github-packages#about-scopes-and-permissions-for-package-registries Signed-off-by: Radoslaw Szwajkowski <[email protected]>
rszwajko
force-pushed
the
keymapPaste
branch
from
5bf7b36 to
45bda28
Compare
General steps: 1. get text from clipboard 2. for each Unicode code point get the keystrokes required to trigger it in the given keyboard layout 3. send the keystrokes to the server using QEMUExtendedKeyEvent Mapping code point to keystrokes: 1. use ucs2keysym() method to convert Unicode code to X11 keysym code. Functionality is part of xorg-server-1.12.2/hw/xquartz/keysym2ucs.c that was ported to TypeScript. 2. map keysym code to mnemonic name using keysym_to_name mapping based on XKB mappings 3. check if there is a mapping for that name in the keymap for the chosen keyboard layout. The keymaps are in Qemu format. In Qemu they are used to enforce a keyboard layout (-k switch). 4. resolved mapping consist of Qemu keycode coressponding to physical key and a list of modifieres i.e. shift, altgr, numlock. Example: 1. assume single char ":" 2. Unicode code point is 0x03a 3. X11 keysym code is 0x03a (Latin-1 set has 1:1 mapping) 4. resolved mnemonic name is "colon" 5. in the keymap en-us name "colon" has following mapping: ['colon', 0x27, 'shift'] 6. based on this mapping we need to: a) press shift key b) press and release key with scancode 0x27 c) release shift key Navigation in the UI: 1. default keymap is en-us 2. if favorite keymaps are defined then the first keymap is used as the default keymap 3. favorite keymaps are stored in the local storage 4. currently selected keymap is stored in the top level component state which allows user to change to Serial Console and back without losing the selection. Reference-Url: https://github.com/xkbcommon/libxkbcommon/blob/b21a58d0cb00c117a4821ac528b586c6d7222f0b/src/keysym.c#L68 Reference-Url: https://github.com/xkbcommon/libxkbcommon/blob/b21a58d0cb00c117a4821ac528b586c6d7222f0b/tools/how-to-type.c#L25 Reference-Url: https://github.com/qemu/qemu/blob/b69801dd6b1eb4d107f7c2f643adf0a4e3ec9124/ui/vnc_keysym.h Reference-Url: https://github.com/qemu/qemu/tree/b69801dd6b1eb4d107f7c2f643adf0a4e3ec9124/pc-bios/keymaps Reference-Url: https://www.x.org/releases/X11R7.7/src/xserver/xorg-server-1.12.2.tar.gz Reference-Url: https://danielhb.github.io/article/2019/05/06/noVNC-QEMU-RFB.html Signed-off-by: Radoslaw Szwajkowski <[email protected]>
rszwajko
force-pushed
the
keymapPaste
branch
from
45bda28 to
d38c84a
Compare
5 participants
Depends on: #3209
📝 Description
General steps:
it in the given keyboard layout
Mapping code point to keystrokes:
Functionality is part of xorg-server-1.12.2/hw/xquartz/keysym2ucs.c
that was ported to TypeScript.
based on XKB mappings
the chosen keyboard layout. The keymaps are in Qemu format.
In Qemu they are used to enforce a keyboard layout (-k switch).
physical key and a list of modifieres i.e. shift, altgr, numlock.
Example:
['colon', 0x27, 'shift']
a) press shift key
b) press and release key with scancode 0x27
c) release shift key
Navigation in the UI:
default keymap
which allows user to change to Serial Console and back without losing
the selection.
Reference-Url: https://github.com/xkbcommon/libxkbcommon/blob/b21a58d0cb00c117a4821ac528b586c6d7222f0b/src/keysym.c#L68
Reference-Url: https://github.com/xkbcommon/libxkbcommon/blob/b21a58d0cb00c117a4821ac528b586c6d7222f0b/tools/how-to-type.c#L25
Reference-Url: https://github.com/qemu/qemu/blob/b69801dd6b1eb4d107f7c2f643adf0a4e3ec9124/ui/vnc_keysym.h
Reference-Url: https://github.com/qemu/qemu/tree/b69801dd6b1eb4d107f7c2f643adf0a4e3ec9124/pc-bios/keymaps
Reference-Url: https://www.x.org/releases/X11R7.7/src/xserver/xorg-server-1.12.2.tar.gz
Reference-Url: https://danielhb.github.io/article/2019/05/06/noVNC-QEMU-RFB.html
Signed-off-by: Radoslaw Szwajkowski [email protected]
🎥 Demo
Demo with US, French and German layouts
Steps in demo:
zaqwertyusingen-uskeymapsudo loadkeys fren-uskeymap - note that the text is malformedScreencast.From.2025-10-02.15-53-24.mp4
Information about unsupported char mapping
Navigation in the UI (favorites and maintaining selection)
Screencast.From.2025-10-02.16-01-06.mp4
Summary by CodeRabbit
New Features
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.