From 085f19555302bad3800d62107d8a9e8902c11421 Mon Sep 17 00:00:00 2001 From: Alireza Date: Wed, 30 Oct 2024 19:36:36 +0100 Subject: [PATCH 01/17] example-app: upgrade monaco editor --- package.json | 4 +- packages/example-app/package.json | 4 +- packages/example-app/src/Editor/Editor.tsx | 42 +-- .../example-app/src/Editor/useEditorTheme.ts | 8 +- yarn.lock | 296 +++++++++++------- 5 files changed, 216 insertions(+), 138 deletions(-) diff --git a/package.json b/package.json index 1d4f84a4..6a23ec75 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "devDependencies": { "@babel/eslint-parser": "^7.18.9", "@microsoft/api-extractor": "^7.25.0", - "@parcel/packager-ts": "^2.8.3", + "@parcel/packager-ts": "2.8.3", "@parcel/transformer-typescript-types": "2.8.3", "@typescript-eslint/eslint-plugin": "^5.30.7", "@typescript-eslint/parser": "^5.30.7", @@ -34,7 +34,7 @@ "eslint-plugin-no-only-tests": "^3.1.0", "eslint-plugin-react": "^7.30.1", "eslint-plugin-react-hooks": "^4.6.0", - "parcel": "^2.8.3", + "parcel": "2.8.3", "prettier": "^2.8.1", "typescript": "4.7.4" }, diff --git a/packages/example-app/package.json b/packages/example-app/package.json index e7a01ec4..d2379640 100644 --- a/packages/example-app/package.json +++ b/packages/example-app/package.json @@ -23,7 +23,7 @@ "dependencies": { "@intellij-platform/core": "workspace:^", "@isomorphic-git/lightning-fs": "^4.6.0", - "@monaco-editor/react": "^4.2.2", + "@monaco-editor/react": "^4.6.0", "@recoiljs/refine": "^0.1.1", "browserfs": "^2.0.0", "caf": "^15.0.0-preB", @@ -32,7 +32,7 @@ "fast-xml-parser": "^4.2.7", "intl-messageformat": "^9.11.2", "isomorphic-git": "^1.24.3", - "monaco-editor": "^0.28.1", + "monaco-editor": "^0.52.0", "pify": "^5.0.0", "ramda": "^0.27.1", "react": "17", diff --git a/packages/example-app/src/Editor/Editor.tsx b/packages/example-app/src/Editor/Editor.tsx index 664a12f5..865ee3c3 100644 --- a/packages/example-app/src/Editor/Editor.tsx +++ b/packages/example-app/src/Editor/Editor.tsx @@ -30,25 +30,29 @@ export const Editor = (props: Omit) => { return ( ); diff --git a/packages/example-app/src/Editor/useEditorTheme.ts b/packages/example-app/src/Editor/useEditorTheme.ts index daa1d5ea..3927aaed 100644 --- a/packages/example-app/src/Editor/useEditorTheme.ts +++ b/packages/example-app/src/Editor/useEditorTheme.ts @@ -34,6 +34,7 @@ export const useEditorTheme = () => { useLayoutEffect(() => { if (monaco) { const name = `jui-theme-${theme.name.replace(" ", "")}`; + const bracketColor = theme.dark ? "#dcdcdc" : "#000"; monaco.editor.defineTheme(name, { base: theme.dark ? "vs-dark" : "vs", inherit: true, @@ -44,12 +45,17 @@ export const useEditorTheme = () => { // FIXME: read from color scheme files. List of available keys: https://github.com/microsoft/monaco-editor/issues/1631 "editor.background": "#2B2B2B", "editor.lineHighlightBackground": "#323232", - "editor.lineHighlightBorder": "none", "editorLineNumber.foreground": "#606366", // LINE_NUMBERS_COLOR "editorActiveLineNumber.foreground": "#a4a3a3", // LINE_NUMBER_ON_CARET_ROW_COLOR "editorIndentGuide.background": "#373737", // INDENT_GUIDE "editorIndentGuide.activeBackground": "#505050", // SELECTED_INDENT_GUIDE "editorGutter.background": theme.dark ? "#313335" : "#f0f0f0", + "editorBracketHighlight.foreground1": bracketColor, + "editorBracketHighlight.foreground2": bracketColor, + "editorBracketHighlight.foreground3": bracketColor, + "editorBracketHighlight.foreground4": bracketColor, + "editorBracketMatch.background": "#43454b", // in new UI + "editorBracketMatch.border": "#00000000", // in new UI } : {}, // token rules for syntax highlighting diff --git a/yarn.lock b/yarn.lock index d8ddc461..1759f013 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4013,69 +4013,68 @@ __metadata: languageName: node linkType: hard -"@monaco-editor/loader@npm:^1.1.1": - version: 1.1.1 - resolution: "@monaco-editor/loader@npm:1.1.1" +"@monaco-editor/loader@npm:^1.4.0": + version: 1.4.0 + resolution: "@monaco-editor/loader@npm:1.4.0" dependencies: state-local: ^1.0.6 peerDependencies: monaco-editor: ">= 0.21.0 < 1" - checksum: 2ee812929ba8e98fbb6ed7edce507439ab3ff97684e713b7e214919a040121fb87c7309faf70b26d14d697209f64536729273e9abc181d83bc582f50fbee4a46 + checksum: 374ec0ea872ee15b33310e105a43217148161480d3955c5cece87d0f801754cd2c45a3f6c539a75da18a066c1615756fb87eaf1003f1df6a64a0cbce5d2c3749 languageName: node linkType: hard -"@monaco-editor/react@npm:^4.2.2": - version: 4.2.2 - resolution: "@monaco-editor/react@npm:4.2.2" +"@monaco-editor/react@npm:^4.6.0": + version: 4.6.0 + resolution: "@monaco-editor/react@npm:4.6.0" dependencies: - "@monaco-editor/loader": ^1.1.1 - prop-types: ^15.7.2 + "@monaco-editor/loader": ^1.4.0 peerDependencies: monaco-editor: ">= 0.25.0 < 1" - react: ^16.8.0 || ^17.0.0 - react-dom: ^16.8.0 || ^17.0.0 - checksum: d6c6764954cf1a09dc69a289f2fab8f26fc2117fa3c092c30648898bfee508005bf576975aa3a81ef0ef480528688c091fdec0b731e5f7239be64d6690d647a1 + react: ^16.8.0 || ^17.0.0 || ^18.0.0 + react-dom: ^16.8.0 || ^17.0.0 || ^18.0.0 + checksum: 9d44e76c5baad6db5f84c90a5540fbd3c9af691b97d76cf2a99b3c8273004d0efe44c2572d80e9d975c9af10022c21e4a66923924950a5201e82017c8b20428c languageName: node linkType: hard -"@msgpackr-extract/msgpackr-extract-darwin-arm64@npm:2.0.2": - version: 2.0.2 - resolution: "@msgpackr-extract/msgpackr-extract-darwin-arm64@npm:2.0.2" +"@msgpackr-extract/msgpackr-extract-darwin-arm64@npm:3.0.3": + version: 3.0.3 + resolution: "@msgpackr-extract/msgpackr-extract-darwin-arm64@npm:3.0.3" conditions: os=darwin & cpu=arm64 languageName: node linkType: hard -"@msgpackr-extract/msgpackr-extract-darwin-x64@npm:2.0.2": - version: 2.0.2 - resolution: "@msgpackr-extract/msgpackr-extract-darwin-x64@npm:2.0.2" +"@msgpackr-extract/msgpackr-extract-darwin-x64@npm:3.0.3": + version: 3.0.3 + resolution: "@msgpackr-extract/msgpackr-extract-darwin-x64@npm:3.0.3" conditions: os=darwin & cpu=x64 languageName: node linkType: hard -"@msgpackr-extract/msgpackr-extract-linux-arm64@npm:2.0.2": - version: 2.0.2 - resolution: "@msgpackr-extract/msgpackr-extract-linux-arm64@npm:2.0.2" +"@msgpackr-extract/msgpackr-extract-linux-arm64@npm:3.0.3": + version: 3.0.3 + resolution: "@msgpackr-extract/msgpackr-extract-linux-arm64@npm:3.0.3" conditions: os=linux & cpu=arm64 languageName: node linkType: hard -"@msgpackr-extract/msgpackr-extract-linux-arm@npm:2.0.2": - version: 2.0.2 - resolution: "@msgpackr-extract/msgpackr-extract-linux-arm@npm:2.0.2" +"@msgpackr-extract/msgpackr-extract-linux-arm@npm:3.0.3": + version: 3.0.3 + resolution: "@msgpackr-extract/msgpackr-extract-linux-arm@npm:3.0.3" conditions: os=linux & cpu=arm languageName: node linkType: hard -"@msgpackr-extract/msgpackr-extract-linux-x64@npm:2.0.2": - version: 2.0.2 - resolution: "@msgpackr-extract/msgpackr-extract-linux-x64@npm:2.0.2" +"@msgpackr-extract/msgpackr-extract-linux-x64@npm:3.0.3": + version: 3.0.3 + resolution: "@msgpackr-extract/msgpackr-extract-linux-x64@npm:3.0.3" conditions: os=linux & cpu=x64 languageName: node linkType: hard -"@msgpackr-extract/msgpackr-extract-win32-x64@npm:2.0.2": - version: 2.0.2 - resolution: "@msgpackr-extract/msgpackr-extract-win32-x64@npm:2.0.2" +"@msgpackr-extract/msgpackr-extract-win32-x64@npm:3.0.3": + version: 3.0.3 + resolution: "@msgpackr-extract/msgpackr-extract-win32-x64@npm:3.0.3" conditions: os=win32 & cpu=x64 languageName: node linkType: hard @@ -4516,7 +4515,7 @@ __metadata: languageName: node linkType: hard -"@parcel/packager-ts@npm:^2.8.3": +"@parcel/packager-ts@npm:2.8.3": version: 2.8.3 resolution: "@parcel/packager-ts@npm:2.8.3" dependencies: @@ -8433,12 +8432,13 @@ __metadata: languageName: node linkType: hard -"@swc/helpers@npm:^0.4.12, legacy-swc-helpers@npm:@swc/helpers@=0.4.14": - version: 0.4.14 - resolution: "@swc/helpers@npm:0.4.14" +"@swc/helpers@npm:^0.4.12": + version: 0.4.37 + resolution: "@swc/helpers@npm:0.4.37" dependencies: + "@swc/legacy-helpers": "npm:@swc/helpers@=0.4.14" tslib: ^2.4.0 - checksum: 273fd3f3fc461a92f3790cc551ea054745c6d6959afbe1232e6d7aa1c722bbc114d308aab96bef5c78fc0303c85c7b472ef00e2253251cc89737f3b1af56e5a5 + checksum: e9577992c74e6f5e94fed4d1e527564031cd1f6e1018c8a9c6fa529edbf9285d20ebeef585a7661e7ba1851df04e087b60e175a7dc88bcec4b99d21fa9dc3116 languageName: node linkType: hard @@ -8470,6 +8470,15 @@ __metadata: languageName: node linkType: hard +"@swc/legacy-helpers@npm:@swc/helpers@=0.4.14, legacy-swc-helpers@npm:@swc/helpers@=0.4.14": + version: 0.4.14 + resolution: "@swc/helpers@npm:0.4.14" + dependencies: + tslib: ^2.4.0 + checksum: 273fd3f3fc461a92f3790cc551ea054745c6d6959afbe1232e6d7aa1c722bbc114d308aab96bef5c78fc0303c85c7b472ef00e2253251cc89737f3b1af56e5a5 + languageName: node + linkType: hard + "@szmarczak/http-timer@npm:^1.1.2": version: 1.1.2 resolution: "@szmarczak/http-timer@npm:1.1.2" @@ -9556,9 +9565,9 @@ __metadata: linkType: hard "abortcontroller-polyfill@npm:^1.1.9": - version: 1.7.3 - resolution: "abortcontroller-polyfill@npm:1.7.3" - checksum: 55739d7f0c9bd6afa2aabb3148778967c4dd4dcff91f6b9259df38da34f9882d3f7730b0954e9767a19cc16a8dd9a58915da4e8a50220300d45af3817d7557b1 + version: 1.7.6 + resolution: "abortcontroller-polyfill@npm:1.7.6" + checksum: f650d15c7a4c1f71a03890e13854b9567052fb1b25d9d32569b9045c310dc3fa45aa5a6607a852ce25fe11d494317b2c278401e656232251340f25f1ae7c8c2b languageName: node linkType: hard @@ -12803,6 +12812,13 @@ __metadata: languageName: node linkType: hard +"detect-libc@npm:^2.0.1": + version: 2.0.3 + resolution: "detect-libc@npm:2.0.3" + checksum: 2ba6a939ae55f189aea996ac67afceb650413c7a34726ee92c40fb0deb2400d57ef94631a8a3f052055eea7efb0f99a9b5e6ce923415daa3e68221f963cfc27d + languageName: node + linkType: hard + "detect-newline@npm:^3.0.0": version: 3.1.0 resolution: "detect-newline@npm:3.1.0" @@ -17557,7 +17573,7 @@ __metadata: dependencies: "@intellij-platform/core": "workspace:^" "@isomorphic-git/lightning-fs": ^4.6.0 - "@monaco-editor/react": ^4.2.2 + "@monaco-editor/react": ^4.6.0 "@recoiljs/refine": ^0.1.1 "@types/diff": ^5.0.9 "@types/jest": ^29.5.2 @@ -17570,7 +17586,7 @@ __metadata: intl-messageformat: ^9.11.2 isomorphic-git: ^1.24.3 jest: ^29.5.0 - monaco-editor: ^0.28.1 + monaco-editor: ^0.52.0 pify: ^5.0.0 ramda: ^0.27.1 react: 17 @@ -17682,80 +17698,98 @@ __metadata: languageName: node linkType: hard -"lightningcss-darwin-arm64@npm:1.19.0": - version: 1.19.0 - resolution: "lightningcss-darwin-arm64@npm:1.19.0" +"lightningcss-darwin-arm64@npm:1.28.2": + version: 1.28.2 + resolution: "lightningcss-darwin-arm64@npm:1.28.2" conditions: os=darwin & cpu=arm64 languageName: node linkType: hard -"lightningcss-darwin-x64@npm:1.19.0": - version: 1.19.0 - resolution: "lightningcss-darwin-x64@npm:1.19.0" +"lightningcss-darwin-x64@npm:1.28.2": + version: 1.28.2 + resolution: "lightningcss-darwin-x64@npm:1.28.2" conditions: os=darwin & cpu=x64 languageName: node linkType: hard -"lightningcss-linux-arm-gnueabihf@npm:1.19.0": - version: 1.19.0 - resolution: "lightningcss-linux-arm-gnueabihf@npm:1.19.0" +"lightningcss-freebsd-x64@npm:1.28.2": + version: 1.28.2 + resolution: "lightningcss-freebsd-x64@npm:1.28.2" + conditions: os=freebsd & cpu=x64 + languageName: node + linkType: hard + +"lightningcss-linux-arm-gnueabihf@npm:1.28.2": + version: 1.28.2 + resolution: "lightningcss-linux-arm-gnueabihf@npm:1.28.2" conditions: os=linux & cpu=arm languageName: node linkType: hard -"lightningcss-linux-arm64-gnu@npm:1.19.0": - version: 1.19.0 - resolution: "lightningcss-linux-arm64-gnu@npm:1.19.0" +"lightningcss-linux-arm64-gnu@npm:1.28.2": + version: 1.28.2 + resolution: "lightningcss-linux-arm64-gnu@npm:1.28.2" conditions: os=linux & cpu=arm64 & libc=glibc languageName: node linkType: hard -"lightningcss-linux-arm64-musl@npm:1.19.0": - version: 1.19.0 - resolution: "lightningcss-linux-arm64-musl@npm:1.19.0" +"lightningcss-linux-arm64-musl@npm:1.28.2": + version: 1.28.2 + resolution: "lightningcss-linux-arm64-musl@npm:1.28.2" conditions: os=linux & cpu=arm64 & libc=musl languageName: node linkType: hard -"lightningcss-linux-x64-gnu@npm:1.19.0": - version: 1.19.0 - resolution: "lightningcss-linux-x64-gnu@npm:1.19.0" +"lightningcss-linux-x64-gnu@npm:1.28.2": + version: 1.28.2 + resolution: "lightningcss-linux-x64-gnu@npm:1.28.2" conditions: os=linux & cpu=x64 & libc=glibc languageName: node linkType: hard -"lightningcss-linux-x64-musl@npm:1.19.0": - version: 1.19.0 - resolution: "lightningcss-linux-x64-musl@npm:1.19.0" +"lightningcss-linux-x64-musl@npm:1.28.2": + version: 1.28.2 + resolution: "lightningcss-linux-x64-musl@npm:1.28.2" conditions: os=linux & cpu=x64 & libc=musl languageName: node linkType: hard -"lightningcss-win32-x64-msvc@npm:1.19.0": - version: 1.19.0 - resolution: "lightningcss-win32-x64-msvc@npm:1.19.0" +"lightningcss-win32-arm64-msvc@npm:1.28.2": + version: 1.28.2 + resolution: "lightningcss-win32-arm64-msvc@npm:1.28.2" + conditions: os=win32 & cpu=arm64 + languageName: node + linkType: hard + +"lightningcss-win32-x64-msvc@npm:1.28.2": + version: 1.28.2 + resolution: "lightningcss-win32-x64-msvc@npm:1.28.2" conditions: os=win32 & cpu=x64 languageName: node linkType: hard "lightningcss@npm:^1.16.1": - version: 1.19.0 - resolution: "lightningcss@npm:1.19.0" + version: 1.28.2 + resolution: "lightningcss@npm:1.28.2" dependencies: detect-libc: ^1.0.3 - lightningcss-darwin-arm64: 1.19.0 - lightningcss-darwin-x64: 1.19.0 - lightningcss-linux-arm-gnueabihf: 1.19.0 - lightningcss-linux-arm64-gnu: 1.19.0 - lightningcss-linux-arm64-musl: 1.19.0 - lightningcss-linux-x64-gnu: 1.19.0 - lightningcss-linux-x64-musl: 1.19.0 - lightningcss-win32-x64-msvc: 1.19.0 + lightningcss-darwin-arm64: 1.28.2 + lightningcss-darwin-x64: 1.28.2 + lightningcss-freebsd-x64: 1.28.2 + lightningcss-linux-arm-gnueabihf: 1.28.2 + lightningcss-linux-arm64-gnu: 1.28.2 + lightningcss-linux-arm64-musl: 1.28.2 + lightningcss-linux-x64-gnu: 1.28.2 + lightningcss-linux-x64-musl: 1.28.2 + lightningcss-win32-arm64-msvc: 1.28.2 + lightningcss-win32-x64-msvc: 1.28.2 dependenciesMeta: lightningcss-darwin-arm64: optional: true lightningcss-darwin-x64: optional: true + lightningcss-freebsd-x64: + optional: true lightningcss-linux-arm-gnueabihf: optional: true lightningcss-linux-arm64-gnu: @@ -17766,9 +17800,11 @@ __metadata: optional: true lightningcss-linux-x64-musl: optional: true + lightningcss-win32-arm64-msvc: + optional: true lightningcss-win32-x64-msvc: optional: true - checksum: c51de34b7379f9da391d0c1157893bb1484357d1ce2212a8c7943690d7a4fed7f2fa0d2dd7a92004b4444662011564ec7bf31f458a1638c856c529fe07285177 + checksum: 6860b65b4352c2bcc3b81bf4950ad754ec431bac89fe44e608325976a096f98985b998a8dda6dc924abb87d0e946e4a8051514ca562d1e453c737184edda4702 languageName: node linkType: hard @@ -18677,10 +18713,10 @@ __metadata: languageName: node linkType: hard -"monaco-editor@npm:^0.28.1": - version: 0.28.1 - resolution: "monaco-editor@npm:0.28.1" - checksum: 4205c12410f154ade2282eee5ad4ca1df1e2ee5e6097e3b18a8606abc535d7ee2f1b54b8ce282db2ba24e8143c6ed4cd06d35dcfb77d457dd9514ca9f0d2e4fd +"monaco-editor@npm:^0.52.0": + version: 0.52.0 + resolution: "monaco-editor@npm:0.52.0" + checksum: 76ab4ea38dfd2bc5687a2d777d7fc14fd7182ac6ed1ed6819b76a47924434bf3a08bcb75c68cd5aff20db86a7633ac46b335a6971ae21a8723a7dcf3e8289931 languageName: node linkType: hard @@ -18719,18 +18755,18 @@ __metadata: languageName: node linkType: hard -"msgpackr-extract@npm:^2.0.2": - version: 2.0.2 - resolution: "msgpackr-extract@npm:2.0.2" - dependencies: - "@msgpackr-extract/msgpackr-extract-darwin-arm64": 2.0.2 - "@msgpackr-extract/msgpackr-extract-darwin-x64": 2.0.2 - "@msgpackr-extract/msgpackr-extract-linux-arm": 2.0.2 - "@msgpackr-extract/msgpackr-extract-linux-arm64": 2.0.2 - "@msgpackr-extract/msgpackr-extract-linux-x64": 2.0.2 - "@msgpackr-extract/msgpackr-extract-win32-x64": 2.0.2 +"msgpackr-extract@npm:^3.0.2": + version: 3.0.3 + resolution: "msgpackr-extract@npm:3.0.3" + dependencies: + "@msgpackr-extract/msgpackr-extract-darwin-arm64": 3.0.3 + "@msgpackr-extract/msgpackr-extract-darwin-x64": 3.0.3 + "@msgpackr-extract/msgpackr-extract-linux-arm": 3.0.3 + "@msgpackr-extract/msgpackr-extract-linux-arm64": 3.0.3 + "@msgpackr-extract/msgpackr-extract-linux-x64": 3.0.3 + "@msgpackr-extract/msgpackr-extract-win32-x64": 3.0.3 node-gyp: latest - node-gyp-build-optional-packages: 5.0.2 + node-gyp-build-optional-packages: 5.2.2 dependenciesMeta: "@msgpackr-extract/msgpackr-extract-darwin-arm64": optional: true @@ -18744,19 +18780,21 @@ __metadata: optional: true "@msgpackr-extract/msgpackr-extract-win32-x64": optional: true - checksum: 6b24c0e89eae881012484787082a50290f78d2dc69df28c28838c65f9cda3f585272c73b9ebbf386f9958c16a9956f0cabddf2ccfc1229ee612a6b88e9519c68 + bin: + download-msgpackr-prebuilds: bin/download-prebuilds.js + checksum: 3b5ae152821feff843380f0b091afbebd80bd224e644f4410abd33d05da3159eb8b0d45c7dcf7d5226ce1d5c71cd68052f066788f46ea7a3cd8791a1c740a079 languageName: node linkType: hard "msgpackr@npm:^1.5.4": - version: 1.6.1 - resolution: "msgpackr@npm:1.6.1" + version: 1.11.2 + resolution: "msgpackr@npm:1.11.2" dependencies: - msgpackr-extract: ^2.0.2 + msgpackr-extract: ^3.0.2 dependenciesMeta: msgpackr-extract: optional: true - checksum: 1c34b1c314e8db2bcb6debb10034bd965f6333812d7a29248745af8e0204cc571484e48bbc09b6cef4e244523acb32db5e9e2637aafbebf03d97bb3898657800 + checksum: 53b30ddd68fd98ae95690017787f3b54414191314f3d36329cc01c073ec35fece87c86de731ef521d1f1b8adeb294008184ad0266d3a0e62cf0867dc728dcbd1 languageName: node linkType: hard @@ -18917,25 +18955,27 @@ __metadata: languageName: node linkType: hard -"node-gyp-build-optional-packages@npm:5.0.2": - version: 5.0.2 - resolution: "node-gyp-build-optional-packages@npm:5.0.2" +"node-gyp-build-optional-packages@npm:5.0.3": + version: 5.0.3 + resolution: "node-gyp-build-optional-packages@npm:5.0.3" bin: - node-gyp-build-optional: optional.js node-gyp-build-optional-packages: bin.js - node-gyp-build-test: build-test.js - checksum: 6fca33cd1e297a446dead8a9bc7a48988be30098c219e75e8466d0218dea6b03bf5da092fe20301cb8b72218356c656422f1a64520c37ebc30eb596b012d1ad9 + node-gyp-build-optional-packages-optional: optional.js + node-gyp-build-optional-packages-test: build-test.js + checksum: be3f0235925c8361e5bc1a03848f5e24815b0df8aa90bd13f1eac91cd86264bbb8b7689ca6cd083b02c8099c7b54f9fb83066c7bb77c2389dc4eceab921f084f languageName: node linkType: hard -"node-gyp-build-optional-packages@npm:5.0.3": - version: 5.0.3 - resolution: "node-gyp-build-optional-packages@npm:5.0.3" +"node-gyp-build-optional-packages@npm:5.2.2": + version: 5.2.2 + resolution: "node-gyp-build-optional-packages@npm:5.2.2" + dependencies: + detect-libc: ^2.0.1 bin: node-gyp-build-optional-packages: bin.js node-gyp-build-optional-packages-optional: optional.js node-gyp-build-optional-packages-test: build-test.js - checksum: be3f0235925c8361e5bc1a03848f5e24815b0df8aa90bd13f1eac91cd86264bbb8b7689ca6cd083b02c8099c7b54f9fb83066c7bb77c2389dc4eceab921f084f + checksum: 3c10d7380901ab5febcd153d2632917fe7507edb15a3405e9ef19801834a4c2162459a67b9944887f737f8718baeb4aaf0002c829a8214011930f2de80e4b42f languageName: node linkType: hard @@ -19317,9 +19357,9 @@ __metadata: linkType: hard "ordered-binary@npm:^1.2.4": - version: 1.2.5 - resolution: "ordered-binary@npm:1.2.5" - checksum: fd0f1322a67064fa7e35bada142b05c50442976907e834a0c4d19b64aa621cd6941fc3d0f87eede91359ace9a932119f3c2248eb43fd52e6103ba1210aa3c506 + version: 1.5.3 + resolution: "ordered-binary@npm:1.5.3" + checksum: 1a118c2ad7b5bcba5c0d512cb702e609779b44f431a7e72974107e4367015f8936e02772012cff92231c2c6872e8f8b35d1829cf7a21d58a795844f162706aec languageName: node linkType: hard @@ -19503,7 +19543,7 @@ __metadata: languageName: node linkType: hard -"parcel@npm:^2.8.3": +"parcel@npm:2.8.3": version: 2.8.3 resolution: "parcel@npm:2.8.3" dependencies: @@ -21501,13 +21541,20 @@ __metadata: languageName: node linkType: hard -"regenerator-runtime@npm:^0.13.3, regenerator-runtime@npm:^0.13.4, regenerator-runtime@npm:^0.13.7": +"regenerator-runtime@npm:^0.13.3, regenerator-runtime@npm:^0.13.4": version: 0.13.10 resolution: "regenerator-runtime@npm:0.13.10" checksum: 09893f5a9e82932642d9a999716b6c626dc53ef2a01307c952ebbf8e011802360163a37c304c18a6c358548be5a72b448e37209954a18696f21e438c81cbb4b9 languageName: node linkType: hard +"regenerator-runtime@npm:^0.13.7": + version: 0.13.11 + resolution: "regenerator-runtime@npm:0.13.11" + checksum: 27481628d22a1c4e3ff551096a683b424242a216fee44685467307f14d58020af1e19660bf2e26064de946bad7eff28950eae9f8209d55723e2d9351e632bbb4 + languageName: node + linkType: hard + "regenerator-runtime@npm:^0.14.0": version: 0.14.0 resolution: "regenerator-runtime@npm:0.14.0" @@ -22007,7 +22054,7 @@ __metadata: dependencies: "@babel/eslint-parser": ^7.18.9 "@microsoft/api-extractor": ^7.25.0 - "@parcel/packager-ts": ^2.8.3 + "@parcel/packager-ts": 2.8.3 "@parcel/transformer-typescript-types": 2.8.3 "@typescript-eslint/eslint-plugin": ^5.30.7 "@typescript-eslint/parser": ^5.30.7 @@ -22021,7 +22068,7 @@ __metadata: eslint-plugin-no-only-tests: ^3.1.0 eslint-plugin-react: ^7.30.1 eslint-plugin-react-hooks: ^4.6.0 - parcel: ^2.8.3 + parcel: 2.8.3 prettier: ^2.8.1 typescript: 4.7.4 languageName: unknown @@ -23447,7 +23494,7 @@ __metadata: languageName: node linkType: hard -"terser@npm:^5.10.0, terser@npm:^5.16.8, terser@npm:^5.2.0": +"terser@npm:^5.10.0, terser@npm:^5.16.8": version: 5.19.2 resolution: "terser@npm:5.19.2" dependencies: @@ -23461,6 +23508,20 @@ __metadata: languageName: node linkType: hard +"terser@npm:^5.2.0": + version: 5.37.0 + resolution: "terser@npm:5.37.0" + dependencies: + "@jridgewell/source-map": ^0.3.3 + acorn: ^8.8.2 + commander: ^2.20.0 + source-map-support: ~0.5.20 + bin: + terser: bin/terser + checksum: 70c06a8ce1288ff4370a7e481beb6fc8b22fc4995371479f49df1552aa9cf8e794ace66e1da6e87057eda1745644311213f5043bda9a06cf55421eff68b3ac06 + languageName: node + linkType: hard + "test-exclude@npm:^6.0.0": version: 6.0.0 resolution: "test-exclude@npm:6.0.0" @@ -24434,7 +24495,14 @@ __metadata: languageName: node linkType: hard -"v8-compile-cache@npm:^2.0.0, v8-compile-cache@npm:^2.0.3": +"v8-compile-cache@npm:^2.0.0": + version: 2.4.0 + resolution: "v8-compile-cache@npm:2.4.0" + checksum: 8eb6ddb59d86f24566503f1e6ca98f3e6f43599f05359bd3ab737eaaf1585b338091478a4d3d5c2646632cf8030288d7888684ea62238cdce15a65ae2416718f + languageName: node + linkType: hard + +"v8-compile-cache@npm:^2.0.3": version: 2.3.0 resolution: "v8-compile-cache@npm:2.3.0" checksum: adb0a271eaa2297f2f4c536acbfee872d0dd26ec2d76f66921aa7fc437319132773483344207bdbeee169225f4739016d8d2dbf0553913a52bb34da6d0334f8e From 4a5f9588f3f656d9e2bf7ee02ba510c492390c23 Mon Sep 17 00:00:00 2001 From: Alireza Date: Wed, 30 Oct 2024 21:19:06 +0100 Subject: [PATCH 02/17] feat(Menu): allow for positioning menu based on a coordinate within viewport --- .../jui/src/Menu/ContextMenuContainer.tsx | 31 +++---- packages/jui/src/Menu/Menu.cy.tsx | 44 ++++++++- packages/jui/src/Menu/MenuOverlay.tsx | 85 +++++++++++++----- .../jui/src/Menu/MenuOverlayFromOrigin.tsx | 62 +++++++++++++ packages/jui/src/Menu/MenuTrigger.tsx | 38 +++----- packages/jui/src/Menu/index.ts | 1 + packages/jui/src/Menu/useContextMenu.tsx | 74 +++------------ .../src/Menu/useOverlayPositionFromOrigin.tsx | 89 +++++++++++++++++++ packages/website/docs/components/Menu.mdx | 70 +++++++++++++++ .../website/src/theme/ReactLiveScope/index.js | 19 ++-- 10 files changed, 375 insertions(+), 138 deletions(-) create mode 100644 packages/jui/src/Menu/MenuOverlayFromOrigin.tsx create mode 100644 packages/jui/src/Menu/useOverlayPositionFromOrigin.tsx diff --git a/packages/jui/src/Menu/ContextMenuContainer.tsx b/packages/jui/src/Menu/ContextMenuContainer.tsx index d9c1e59f..c834ad65 100644 --- a/packages/jui/src/Menu/ContextMenuContainer.tsx +++ b/packages/jui/src/Menu/ContextMenuContainer.tsx @@ -4,7 +4,7 @@ import { useMenuTriggerState } from "@react-stately/menu"; import { OverlayTriggerProps } from "@react-types/overlays"; import { useContextMenu, UseContextMenuProps } from "./useContextMenu"; -import { MenuOverlay } from "./MenuOverlay"; +import { MenuOverlayFromOrigin } from "@intellij-platform/core/Menu/MenuOverlayFromOrigin"; interface ContextMenuContainerProps extends Omit, "children">, @@ -40,7 +40,7 @@ export const ContextMenuContainer = React.forwardRef( ) => { const state = useMenuTriggerState({} as OverlayTriggerProps); - const { overlayProps, containerProps, overlayRef } = useContextMenu( + const { positionOrigin, containerProps, overlayRef } = useContextMenu( { onOpen, isDisabled }, state ); @@ -54,19 +54,20 @@ export const ContextMenuContainer = React.forwardRef( {children} )} - - {renderMenu()} - + {state.isOpen && ( + + {renderMenu()} + + )} ); } diff --git a/packages/jui/src/Menu/Menu.cy.tsx b/packages/jui/src/Menu/Menu.cy.tsx index b74135dd..a4e92f8c 100644 --- a/packages/jui/src/Menu/Menu.cy.tsx +++ b/packages/jui/src/Menu/Menu.cy.tsx @@ -2,6 +2,7 @@ import { composeStories } from "@storybook/react"; import * as React from "react"; import * as stories from "./Menu.stories"; import { + ContextMenuContainer, Divider, Item, Menu, @@ -725,6 +726,14 @@ describe("Menu with trigger", () => { matchImageSnapshot(`menu-with-trigger--position-${num}`); } }); + + it("closes when right clicking outside", () => { + // TODO: fix the issue! + cy.mount(); + cy.get("button[aria-haspopup]").click(); // open the menu by clicking the trigger. + cy.get("body").rightclick("bottomRight"); + cy.findByRole("menu").should("not.exist"); + }); }); describe("ContextMenu", () => { @@ -815,6 +824,27 @@ describe("ContextMenu", () => { cy.findByRole("menu").should("not.exist"); }); + it("is closed when right clicking on another context menu trigger area", () => { + const renderMenu = () => ( + + Menu item + + ); + cy.mount( + <> + + Container 1 + + + Container 2 + + + ); + cy.contains("Container 1").rightclick(); + cy.contains("Container 2").rightclick("left"); + cy.findAllByRole("menu").should("have.length", 1); + }); + it("is closed after an action is triggered", () => { cy.mount(); cy.scrollTo("bottom", { duration: 0 }); @@ -842,11 +872,19 @@ describe("ContextMenu", () => { it("lets user select nested menu items by mouse", () => { const onAction = cy.stub(); cy.mount(); - cy.get("#context-menu-container").rightclick("top", { - scrollBehavior: false, - }); + cy.get("#context-menu-container") + // Not sure why but this extra click here became + // necessary for test to pass, after some refactoring. + // Without it, clicking "Show History" doesn't work, and realClicking it + // will also not trigger onAction, only the first time the menu is opened. + // The issue wasn't reproducible in real interaction. + .click() + .rightclick("top", { + scrollBehavior: false, + }); cy.findByRole("menuitem", { name: "Local History" }).click(); cy.findByRole("menuitem", { name: "Show History" }).click(); + cy.wrap(onAction).should("be.calledOnce"); }); }); diff --git a/packages/jui/src/Menu/MenuOverlay.tsx b/packages/jui/src/Menu/MenuOverlay.tsx index 563fdd18..8d77c317 100644 --- a/packages/jui/src/Menu/MenuOverlay.tsx +++ b/packages/jui/src/Menu/MenuOverlay.tsx @@ -1,48 +1,87 @@ -import React, { HTMLProps } from "react"; -import { MenuTriggerState } from "@react-stately/menu"; +import React, { HTMLProps, useEffect } from "react"; import { FocusScope } from "@intellij-platform/core/utils/FocusScope"; import { MenuOverlayContext, MenuProps, } from "@intellij-platform/core/Menu/Menu"; -import { Overlay } from "@intellij-platform/core/Overlay"; +import { areInNestedOverlays, Overlay } from "@intellij-platform/core/Overlay"; +import { mergeProps, useObjectRef } from "@react-aria/utils"; +import { useOverlay, usePreventScroll } from "@react-aria/overlays"; + +export interface MenuOverlayProps { + children: React.ReactNode; + restoreFocus?: boolean; + overlayProps: HTMLProps; + overlayRef?: React.Ref; + /** + * Sets the default value of {@link Menu}'s {@link MenuProps#autoFocus} prop. + */ + defaultAutoFocus?: MenuProps["autoFocus"]; + onClose: () => void; +} /** - * Overlay container for menu. Extracted into a separate component, to be used by components like MenuTrigger or - * ContextMenuContainer, that need to render a menu as an overlay. - * @private + * Overlay container for Menu. + * Positioning is not implemented at this layer. + * {@link MenuOverlayProps#overlayProps} should be used for positioning. */ export function MenuOverlay({ children, restoreFocus, - overlayProps, - overlayRef, + overlayProps: otherOverlayProps, + overlayRef: inputOverlayRef, defaultAutoFocus, - state, -}: { - children: React.ReactNode; - restoreFocus?: boolean; - overlayProps: HTMLProps; - overlayRef: React.Ref; + onClose, +}: MenuOverlayProps) { + const overlayRef = useObjectRef(inputOverlayRef); + const { overlayProps } = useOverlay( + { + onClose, + shouldCloseOnBlur: false, + isOpen: true, + isKeyboardDismissDisabled: false, + isDismissable: true, + shouldCloseOnInteractOutside: (element) => { + // FIXME: this is kind of hacky and should be removed when nested menu is properly supported + return !areInNestedOverlays(overlayRef.current, element); + }, + }, + overlayRef + ); + + usePreventScroll(); + /** - * Sets the default value of {@link Menu}'s {@link MenuProps#autoFocus} prop. + * right clicks outside are not currently captured as "outside interaction" by react-aria's useOverlay hook. + * so we set up a global listener to close the context menu when contextmenu event is triggered outside the + * context menu container. + * NOTE: event handler is set up for the `capture` phase, to have it run before the handler for context menu + * when the menu is used as a context menu */ - defaultAutoFocus?: MenuProps["autoFocus"]; - state: MenuTriggerState; -}) { - if (!state.isOpen) { - return null; - } + useEffect(() => { + const onOutsideContextMenu = () => { + onClose(); + }; + document.addEventListener("contextmenu", onOutsideContextMenu, { + capture: true, + }); + return () => + document.removeEventListener("contextmenu", onOutsideContextMenu); + }, []); + return ( -
+
{children}
diff --git a/packages/jui/src/Menu/MenuOverlayFromOrigin.tsx b/packages/jui/src/Menu/MenuOverlayFromOrigin.tsx new file mode 100644 index 00000000..3edd35f5 --- /dev/null +++ b/packages/jui/src/Menu/MenuOverlayFromOrigin.tsx @@ -0,0 +1,62 @@ +import React, { ForwardedRef } from "react"; +import { + MenuOverlay, + MenuOverlayProps, +} from "@intellij-platform/core/Menu/MenuOverlay"; +import { useOverlayPositionFromOrigin } from "@intellij-platform/core/Menu/useOverlayPositionFromOrigin"; +import { useObjectRef } from "@react-aria/utils"; + +interface MenuOverlayFromOriginProps + extends Pick { + /** + * Origin point within the viewport, based on which the menu overlay should be positioned. + * Any pointer/mouse event, or a plain object with clientX and clientY can be passed. + */ + origin: + | { + /** + * Horizontal coordinate of the origin point within the viewport. + * See {@link MouseEvent.clientX} + */ + clientX: number; + /** + * Vertical coordinate of the origin point within the viewport. + * See {@link MouseEvent.clientX} + */ + clientY: number; + } + | undefined; + children: React.ReactNode; +} + +/** + * Menu overlay position based on an origin point on the screen. + * Useful when the menu is opened by a pointer event. + */ +export const MenuOverlayFromOrigin = React.forwardRef( + function MenuOverlayFromOrigin( + { children, origin, ...otherProps }: MenuOverlayFromOriginProps, + forwardedRef: ForwardedRef + ) { + const overlayRef = useObjectRef(forwardedRef); + const { positionProps } = useOverlayPositionFromOrigin({ + overlayRef, + origin, + containerPadding: { x: 0, y: 4 }, + }); + return ( + <> + {Boolean(origin) && ( + + {children} + + )} + + ); + } +); diff --git a/packages/jui/src/Menu/MenuTrigger.tsx b/packages/jui/src/Menu/MenuTrigger.tsx index 3440b11b..32873ecc 100644 --- a/packages/jui/src/Menu/MenuTrigger.tsx +++ b/packages/jui/src/Menu/MenuTrigger.tsx @@ -1,8 +1,7 @@ import React, { HTMLAttributes, RefObject } from "react"; import { useButton } from "@react-aria/button"; import { AriaMenuOptions, useMenuTrigger } from "@react-aria/menu"; -import { useOverlay, useOverlayPosition } from "@react-aria/overlays"; -import { mergeProps } from "@react-aria/utils"; +import { useOverlayPosition } from "@react-aria/overlays"; import { useMenuTriggerState } from "@react-stately/menu"; import { MenuTriggerProps as AriaMenuTriggerProps } from "@react-types/menu"; @@ -89,22 +88,6 @@ export const MenuTrigger: React.FC = ({ preventFocusOnPress, }; const { buttonProps } = useButton(ariaButtonProps, triggerRef); - const { overlayProps } = useOverlay( - { - onClose: () => { - return state.close(); - }, - shouldCloseOnBlur: false, - isOpen: state.isOpen, - isKeyboardDismissDisabled: false, - isDismissable: true, - shouldCloseOnInteractOutside: (element) => { - // FIXME: this is kind of hacky and should be removed when nested menu is properly supported - return !element.matches("[role=menu] *"); - }, - }, - overlayRef - ); const { overlayProps: positionProps } = useOverlayPosition({ targetRef: positioningTargetRef ?? triggerRef, @@ -113,20 +96,23 @@ export const MenuTrigger: React.FC = ({ shouldFlip, offset: 0, containerPadding: 0, + onClose: () => state.close(), isOpen: state.isOpen, }); return ( <> {children(buttonProps, triggerRef)} - - {renderMenu({ menuProps })} - + {state.isOpen && ( + + {renderMenu({ menuProps })} + + )} ); }; diff --git a/packages/jui/src/Menu/index.ts b/packages/jui/src/Menu/index.ts index 49de9c68..e0d0e3aa 100644 --- a/packages/jui/src/Menu/index.ts +++ b/packages/jui/src/Menu/index.ts @@ -3,6 +3,7 @@ export { SpeedSearchMenu, type SpeedSearchMenuProps } from "./SpeedSearchMenu"; export { MenuTrigger, type MenuTriggerProps } from "./MenuTrigger"; export { MenuItemLayout } from "./MenuItemLayout"; export { ContextMenuContainer } from "./ContextMenuContainer"; +export { MenuOverlayFromOrigin } from "./MenuOverlayFromOrigin"; // Collection components are public API of Menu too, but not re-exported because of https://github.com/parcel-bundler/parcel/issues/4399 // export * from "../Collections"; diff --git a/packages/jui/src/Menu/useContextMenu.tsx b/packages/jui/src/Menu/useContextMenu.tsx index 2639416a..99cefe60 100644 --- a/packages/jui/src/Menu/useContextMenu.tsx +++ b/packages/jui/src/Menu/useContextMenu.tsx @@ -1,9 +1,5 @@ -import React, { useEffect, useRef } from "react"; +import React, { useRef, useState } from "react"; import { MenuTriggerState } from "@react-stately/menu"; -import { useOverlay } from "@react-aria/overlays"; -import { mergeProps } from "@react-aria/utils"; -import { useMouseEventOverlayPosition } from "@intellij-platform/core/utils/useMouseEventOverlayPosition"; -import { areInNestedOverlays } from "@intellij-platform/core/Overlay"; export type UseContextMenuProps = { /** @@ -28,25 +24,27 @@ export const useContextMenu = ( { isDisabled = false, onOpen }: UseContextMenuProps, state: MenuTriggerState ) => { - const containerRef = useRef(null); + const [positionOrigin, setPositionOrigin] = useState(); /** * NOTE: not using useMenuTrigger because: * - There is no option to have a trigger like this: "right click + long press only by touch" which seems to be the * reasonable trigger for context menu. If we want to use it just for long press, we could disable it if it's not * a touch device, but that would be suboptimal, since both touch and mouse can be available, and it should depend * not on availability of touch, but on the triggered event type. Plus, isDisabled is broken in v<3.5.0 - * - It's not quite clear at the moment, if the aria attributes that useMenuTrigger sets would be applicable in case - * of this context menu component too. the trigger is not the container. For example, if there is a list rendered + * - It's not quite clear at the moment, if the aria attributes that useMenuTrigger sets would be applicable to + * this context menu component too. The trigger is not the container. For example, if there is a list rendered * inside, the selected item would be the trigger. Maybe even this component, as a container for context menu - * is not the best way to allow for context menu, when comes to a11y concerns. For now, we skip a11y props of the + * is not the best way to allow for context menu, when it comes to a11y concerns. For now, we skip a11y props of the * trigger. A11y props of the menu itself (e.g. aria-label) would also be up to the usage of this component. * * TODO: add support for long touch */ const onContextMenu = (e: React.MouseEvent) => { - containerRef.current = e.timeStamp; - updatePosition(e); - onOpen?.({ target: e.target as Element }); + if (!(e.target instanceof Element)) { + return; + } + setPositionOrigin(e); + onOpen?.({ target: e.target }); e.preventDefault(); // NOTE: we can't use offsetX/offsetY, because it would depend on the exact target that was clicked. if (state.isOpen) { @@ -63,55 +61,9 @@ export const useContextMenu = ( state.open(null); } }; - useEffect(() => { - const onOutsideContextMenu = (e: MouseEvent) => { - // Using timestamp an easy (and hopefully reliable) way to detect if it's the same - // event being handled by onContextMenu, avoiding the overhead of requiring a ref for the container. - if (containerRef.current !== e.timeStamp) { - state.close(); - } - }; - /** - * right clicks outside are not currently captured as "outside interaction" by react-aria's useOverlay hook. - * so we set up a global listener to close the context menu when contextmenu event is triggered outside the - * context menu container. - * to not require a ref just for this, the ref is manually updated when contextmenu event is triggered - * on the container (which happens before the event propagates to the document). - */ - document.addEventListener("contextmenu", onOutsideContextMenu); - return () => - document.removeEventListener("contextmenu", onOutsideContextMenu); - }, []); const overlayRef = useRef(null); - const { overlayProps: positionProps, updatePosition } = - useMouseEventOverlayPosition({ - overlayRef, - placement: "bottom start", - // shouldFlip should be false, but it doesn't work as expected. Overlay container is rendered within the view port - // but the menu overflows from the overlay container - shouldFlip: true, - offset: -8, - crossOffset: 2, // to not get the first item hovered on open - isOpen: state.isOpen, - }); - const { overlayProps } = useOverlay( - { - onClose: () => { - return state.close(); - }, - shouldCloseOnBlur: false, - isOpen: state.isOpen, - isKeyboardDismissDisabled: false, - isDismissable: true, - shouldCloseOnInteractOutside: (element) => { - return !areInNestedOverlays(overlayRef.current, element); - }, - }, - overlayRef - ); - const containerProps: React.HTMLAttributes = isDisabled ? {} : { @@ -127,9 +79,7 @@ export const useContextMenu = ( * react-aria hooks, but it seemed unnecessary here. */ overlayRef, - /** - * props to be applied on the menu overlay wrapper. - */ - overlayProps: mergeProps(overlayProps, positionProps), + + positionOrigin, }; }; diff --git a/packages/jui/src/Menu/useOverlayPositionFromOrigin.tsx b/packages/jui/src/Menu/useOverlayPositionFromOrigin.tsx new file mode 100644 index 00000000..00d5c139 --- /dev/null +++ b/packages/jui/src/Menu/useOverlayPositionFromOrigin.tsx @@ -0,0 +1,89 @@ +import React, { RefObject, useLayoutEffect, useState } from "react"; + +/** + * Similar to {@link import('@react-aria/overlays').useOverlayPosition useOverlayPosition}, + * but for positioning an overlay relative to a point, typically coordinates of a mouse event. + * It's less advanced than useOverlayPosition, not taking into account many edge cases that + * `useOverlayPosition` does. + * Most importantly: + * - Window resize is not taken into account + * - Overlay resize is not taken into account. + * Positioning only happens when the origin changes. + * - There are no options for positioning options like placement, offset or crossOffset + * + * @see https://github.com/adobe/react-spectrum/discussions/7267 + */ +export function useOverlayPositionFromOrigin({ + overlayRef, + origin, + containerPadding, +}: { + overlayRef: RefObject; + containerPadding?: number | { x: number; y: number }; + origin: Pick | undefined; +}) { + const [position, setPosition] = useState<{ left?: number; top?: number }>({}); + + useLayoutEffect(() => { + const overlayElement = overlayRef.current; + setPosition( + origin && overlayElement + ? calculatePosition({ + clientX: + origin.clientX + + 1.5 /* a tiny offset is added to avoid menu items getting hovered upon open. 1px doesn't consistently work. */, + clientY: origin.clientY, + containerPadding, + overlayWidth: overlayElement.offsetWidth, + overlayHeight: overlayElement.offsetHeight, + }) + : {} + ); + }, [origin?.clientX, origin?.clientY]); + return { + positionProps: { + style: { position: "fixed", zIndex: 100000, ...position } as const, + }, + }; +} + +function calculatePosition({ + clientX, + clientY, + overlayWidth, + overlayHeight, + containerPadding = 0, +}: { + clientX: number; + clientY: number; + overlayWidth: number; + overlayHeight: number; + containerPadding?: number | { x: number; y: number }; +}) { + const totalWidth = + document.documentElement.clientWidth - + (typeof containerPadding === "object" + ? containerPadding.x + : containerPadding); + const totalHeight = + document.documentElement.clientHeight - + (typeof containerPadding === "object" + ? containerPadding.y + : containerPadding); + + let top = clientY; + let left = clientX; + + if (left + overlayWidth > totalWidth) { + left = totalWidth - overlayWidth; + } + + if (top + overlayHeight > totalHeight) { + top = totalHeight - overlayHeight; + } + + top = Math.max(0, top); + left = Math.max(0, left); + + return { top, left }; +} diff --git a/packages/website/docs/components/Menu.mdx b/packages/website/docs/components/Menu.mdx index 8d4ef90b..21f211a7 100644 --- a/packages/website/docs/components/Menu.mdx +++ b/packages/website/docs/components/Menu.mdx @@ -316,6 +316,76 @@ just by composition of those components and `ContextMenuContainer`. A caveat to element that will be added if you want context menu, which may need some styling to have no effect on the layout. ::: +### MenuOverlayFromOrigin + +`ContextMenuContainer` uses `MenuOverlayFromOrigin` under the hood, +to position the menu overlay based on the contextmenu event's coordinate. +For use cases where `ContextMenuContainer` can't be used, `MenuOverlayFromOrigin` can be used directly +to place a menu in an overlay that's positioned based on a pointer event, which can be used to implement contextmenu. + +```tsx live noPadding +function ContextMenuExample() { + const [origin, setOrigin] = React.useState(null); + return ( + <> + { + monacoEditor.focus(); + monacoEditor.onContextMenu((c) => { + c.event.preventDefault(); + Promise.resolve().then(() => { + setOrigin(c.event.browserEvent); + }); + }); + }} + > + {origin && ( + setOrigin(null)} + > + + + } + content="Copy" + shortcut={"⌘C"} + /> + + + } + content="Paste" + shortcut={"⌘V"} + /> + + + + Finder + Terminal + + } + content="Github" + /> + + + + + )} + + ); +} +``` + ## Submenu Behaviour By default, menu items with submenu open the submenu on hover. Pressing such items also opens the submenu, if not diff --git a/packages/website/src/theme/ReactLiveScope/index.js b/packages/website/src/theme/ReactLiveScope/index.js index 2cbe41c9..ff2b3c5a 100644 --- a/packages/website/src/theme/ReactLiveScope/index.js +++ b/packages/website/src/theme/ReactLiveScope/index.js @@ -5,29 +5,30 @@ * LICENSE file in the root directory of this source tree. */ +import React from "react"; import * as juiComponents from "@intellij-platform/core"; import darculaThemeJson from "@intellij-platform/core/themes/darcula.theme.json"; import lightThemeJson from "@intellij-platform/core/themes/intellijlaf.theme.json"; import highContrastThemeJson from "@intellij-platform/core/themes/HighContrast.theme.json"; -const LazyExampleApp = React.lazy(() => import("jui-example-app/src/App")); -const ExampleApp = () => ( - // Because ReactLive doesn't render a Suspense around what it renders. - - - -); -import React from "react"; +const WithLoading = (Component) => (props) => + ( + // Because ReactLive doesn't render a Suspense around what it renders. + + + + ); // Add react-live imports you need here const ReactLiveScope = { React, ...React, ...juiComponents, - ExampleApp, + ExampleApp: WithLoading(React.lazy(() => import("jui-example-app/src/App"))), darculaThemeJson, lightThemeJson, highContrastThemeJson, + MonacoEditor: WithLoading(React.lazy(() => import("@monaco-editor/react"))), }; export default ReactLiveScope; From 5c8eb056133706e97975ae48cc5e73f6fe989f37 Mon Sep 17 00:00:00 2001 From: Alireza Date: Thu, 31 Oct 2024 22:43:14 +0100 Subject: [PATCH 03/17] feat(ActionsMenu): allow for dividers in action group definition feat(example-app): add editor context menu --- packages/example-app/package.json | 1 + .../example-app/src/Editor/FileEditor.tsx | 308 ++++++++++-------- .../example-app/src/Editor/editorActionIds.ts | 18 + .../src/Editor/useEditorActionGroup.tsx | 259 +++++++++++++++ .../src/Project/useProjectActions.tsx | 2 +- packages/example-app/src/appActionIds.ts | 4 + packages/example-app/src/exampleAppKeymap.ts | 144 +++++++- packages/jui/src/ActionSystem/ActionGroup.tsx | 5 +- .../jui/src/ActionSystem/ActionsProvider.tsx | 100 +++--- .../ActionSystem/components/ActionsMenu.tsx | 5 +- .../useCreateDefaultActionGroup.tsx | 29 +- .../jui/src/ActionSystem/shortcutToString.ts | 4 + packages/jui/src/Menu/renderMenuNodes.tsx | 5 +- 13 files changed, 700 insertions(+), 184 deletions(-) create mode 100644 packages/example-app/src/Editor/editorActionIds.ts create mode 100644 packages/example-app/src/Editor/useEditorActionGroup.tsx create mode 100644 packages/example-app/src/appActionIds.ts diff --git a/packages/example-app/package.json b/packages/example-app/package.json index d2379640..91d2784e 100644 --- a/packages/example-app/package.json +++ b/packages/example-app/package.json @@ -11,6 +11,7 @@ "build": "../../node_modules/.bin/parcel build" }, "source": "src/index.html", + "//alias": "Nested imports from @intellij-platform/core/* are not a part of public API and are listed individually here, until it's clearer if they should be a part of the public API", "alias": { "caf": "caf/dist/esm/index.mjs", "@intellij-platform/core/utils/tree-utils": "@intellij-platform/core/src/utils/tree-utils", diff --git a/packages/example-app/src/Editor/FileEditor.tsx b/packages/example-app/src/Editor/FileEditor.tsx index 549ca8b2..4e2ebac0 100644 --- a/packages/example-app/src/Editor/FileEditor.tsx +++ b/packages/example-app/src/Editor/FileEditor.tsx @@ -1,5 +1,7 @@ import { Monaco } from "@monaco-editor/react"; import { + ActionGroupMenu, + ActionsProvider, ActionTooltip, ContextMenuContainer, EditorTabContent, @@ -8,12 +10,14 @@ import { Item, Menu, MenuItemLayout, + MenuOverlayFromOrigin, PlatformIcon, styled, TabCloseButton, TabItem, TooltipTrigger, useAction, + useActionGroup, useLatest, } from "@intellij-platform/core"; import { editor, languages } from "monaco-editor"; @@ -37,6 +41,7 @@ import { useActivePathsProvider } from "../Project/project.state"; import { notImplemented } from "../Project/notImplemented"; import { useExistingLatestRecoilValue } from "../recoil-utils"; import { EditorZeroState } from "./EditorZeroState"; +import { useEditorActionGroup } from "./useEditorActionGroup"; const editorFullState = selector({ key: "editorState", @@ -58,6 +63,8 @@ export const FileEditor = () => { const [editorTabs, editorStateManager] = useEditorState(); const editorRef = useRef(); const [active, setActive] = useState(false); + const [contextMenu, setContextMenu] = + useState(null); const hideAllAction = useAction(HIDE_ALL_WINDOWS_ACTION_ID); const setCursorPositionState = useSetRecoilState(editorCursorPositionState); @@ -79,6 +86,8 @@ export const FileEditor = () => { const setContent = useSetRecoilState(fileContentState(filePath)); const updateFileStatus = useRefreshFileStatus(); + const editorActionGroupDefinition = useEditorActionGroup(editorRef); + const updateContent = (newContent: string = "") => { setActive(false); setContent(newContent); @@ -98,144 +107,187 @@ export const FileEditor = () => { ); return ( - { - setActive(true); - setEditorRef({ - focus: () => editorRef.current?.focus(), - }); - }, - })} - > - {editorTabs.length > 0 ? ( - <> - ( - - Close - Close Other tabs - Close all tabs - Close tabs to the left - Close tabs to the right - - )} - > - { - editorStateManager.select( - editorTabs.findIndex((tab) => tab.filePath === key) - ); - }} - noBorders - > - {(tab) => { - const filename = path.basename(tab.filePath); - const icon = ( - - ); - return ( - - } + + {({ shortcutHandlerProps }) => ( + { + setActive(true); + setEditorRef({ + focus: () => editorRef.current?.focus(), + }); + }, + })} + > + {editorTabs.length > 0 ? ( + <> + ( + - } - > - - {filename} - + Close + Close Other tabs + Close all tabs + Close tabs to the left + Close tabs to the right + + )} + > + { + editorStateManager.select( + editorTabs.findIndex((tab) => tab.filePath === key) + ); + }} + noBorders + > + {(tab) => { + const filename = path.basename(tab.filePath); + const icon = ( + + ); + return ( + } - closeButton={ - + > + } + > + + {filename} + } - > - { - if (e.altKey) { - tabActionsRef.current.closeOthersTabs( - editorTabs.indexOf(tab) - ); - } else { - tabActionsRef.current.closePath(tab.filePath); + closeButton={ + } - }} - /> - - } - containerProps={{ - onDoubleClick: () => { - hideAllAction?.perform(); - }, - }} - /> - - - ); - }} - - - {typeof content === "string" ? ( - /** - * ## Note - * TLDR: Keeping the editor mounted when filePath changes is intentional and does matter. - * - * Whether the editor component is kept mounted as tabs change or not has nuances that can lead to minor - * focus issues. For example calling editorStateManager.focus() may do nothing if the editor is unmounted due - * to filePath being changed. focusing editor in createFile action is an example of such case. It also affects - * certain focus management code within the FileEditor. For example, focusing the editor on tab changes will - * be necessary if the editor remounts with each file change. Or autofocus behavior of the editor can be - * done on the onMount callback of the Editor component, if it's only mounted once. But the same code leads - * to focus issues if the editor is mounted on active tab changes, when they are not done via the tab UI, - * but as a side effect of another action like opening a file via Project tool window. - * - */ - { - monacoEditor.focus(); - enableJsx(monaco); - editorRef.current = monacoEditor; - monacoEditor.onDidChangeCursorPosition((e) => { - setCursorPositionState(e.position); - }); - monacoEditor.onDidChangeModel(() => { - // TODO: set the editor tab state, and add an atom effect to persist whole editor state across loads - }); - }} - onChange={updateContent} - value={content ?? ""} - /> + > + { + if (e.altKey) { + tabActionsRef.current.closeOthersTabs( + editorTabs.indexOf(tab) + ); + } else { + tabActionsRef.current.closePath( + tab.filePath + ); + } + }} + /> + + } + containerProps={{ + onDoubleClick: () => { + hideAllAction?.perform(); + }, + }} + /> + + + ); + }} + + + {typeof content === "string" ? ( + /** + * ## Note + * TLDR: Keeping the editor mounted when filePath changes is intentional and does matter. + * + * Whether the editor component is kept mounted as tabs change or not has nuances that can lead to minor + * focus issues. For example calling editorStateManager.focus() may do nothing if the editor is unmounted due + * to filePath being changed. focusing editor in createFile action is an example of such case. It also affects + * certain focus management code within the FileEditor. For example, focusing the editor on tab changes will + * be necessary if the editor remounts with each file change. Or autofocus behavior of the editor can be + * done on the onMount callback of the Editor component, if it's only mounted once. But the same code leads + * to focus issues if the editor is mounted on active tab changes, when they are not done via the tab UI, + * but as a side effect of another action like opening a file via Project tool window. + * + */ + { + monacoEditor.focus(); + monacoEditor.onContextMenu((c) => { + c.event.preventDefault(); + // opening contextmenu async, because otherwise the editor takes the focus back + // from the menu, right after it's opened. + Promise.resolve().then(() => { + setContextMenu(c); + }); + }); + enableJsx(monaco); + editorRef.current = monacoEditor; + monacoEditor.onDidChangeCursorPosition((e) => { + setCursorPositionState(e.position); + }); + monacoEditor.onDidChangeModel(() => { + // TODO: set the editor tab state, and add an atom effect to persist whole editor state across loads + }); + }} + onChange={updateContent} + value={content ?? ""} + /> + ) : ( + content && "UNSUPPORTED CONTENT" + )} + ) : ( - content && "UNSUPPORTED CONTENT" + )} - - ) : ( - + {loadingState === "loading" && } + {contextMenu && ( + setContextMenu(null)} + > + + + )} + )} - {loadingState === "loading" && } - + ); }; +function EditorActionGroupMenu() { + const actionGroup = useActionGroup("EditorPopupMenu"); + if (!actionGroup) { + return null; // Replace with a placeholder "Nothing to show" menu + } + return ( + + {(menuProps) => ( + + )} + + ); +} + const StyledFileEditorContainer = styled.div` position: relative; height: 100%; diff --git a/packages/example-app/src/Editor/editorActionIds.ts b/packages/example-app/src/Editor/editorActionIds.ts new file mode 100644 index 00000000..a80174f2 --- /dev/null +++ b/packages/example-app/src/Editor/editorActionIds.ts @@ -0,0 +1,18 @@ +export const editorActionIds = { + EXPAND_REGION: "ExpandRegion", + EXPAND_REGION_RECURSIVELY: "ExpandRegionRecursively", + EXPAND_ALL_REGIONS: "ExpandAllRegions", + EXPAND_ALL_TO_LEVEL: "ExpandToLevel", + EXPAND_ALL_TO_LEVEL1: "ExpandToLevel1", + EXPAND_ALL_TO_LEVEL2: "ExpandToLevel2", + EXPAND_ALL_TO_LEVEL3: "ExpandToLevel3", + EXPAND_ALL_TO_LEVEL4: "ExpandToLevel4", + EXPAND_ALL_TO_LEVEL5: "ExpandToLevel5", + COLLAPSE_REGION: "CollapseRegion", + COLLAPSE_REGION_RECURSIVELY: "CollapseRegionRecursively", + COLLAPSE_ALL_REGIONS: "CollapseAllRegions", + COLLAPSE_DOC_COMMENTS: "CollapseDocComments", + TOGGLE_FOLDING: "ToggleFolding", + EDITOR_LANG_POPUP_MENU: "EditorLangPopupMenu", + CompareClipboardWithSelection: "CompareClipboardWithSelection", +}; diff --git a/packages/example-app/src/Editor/useEditorActionGroup.tsx b/packages/example-app/src/Editor/useEditorActionGroup.tsx new file mode 100644 index 00000000..1487c3ab --- /dev/null +++ b/packages/example-app/src/Editor/useEditorActionGroup.tsx @@ -0,0 +1,259 @@ +import React, { MutableRefObject } from "react"; +import copyToClipboard from "clipboard-copy"; +import { editor } from "monaco-editor"; +import { + ActionContext, + ActionGroupDefinition, + CommonActionId, + PlatformIcon, + useCreateDefaultActionGroup, +} from "@intellij-platform/core"; +import { notNull } from "@intellij-platform/core/utils/array-utils"; + +import { notImplemented } from "../Project/notImplemented"; +import { AppActionIds } from "../appActionIds"; +import { editorActionIds } from "./editorActionIds"; + +export function useEditorActionGroup( + editorRef: MutableRefObject +): ActionGroupDefinition { + const createDefaultActionGroup = useCreateDefaultActionGroup(); + const editor = editorRef.current; + const selection = editor?.getSelection(); + + const selectedText = selection // FIXME: needs to become reactive + ? editor?.getModel()?.getValueInRange(selection) + : ""; + + return createDefaultActionGroup({ + id: "EditorPopupMenu", + title: "Editor Popup Menu", + description: "Editor Popup Menu", + children: ( + [ + createDefaultActionGroup({ + id: "ShowIntentionsGroup", + title: "Show Intentions Group", + isSearchable: false, + menuPresentation: "section", + children: [ + { + id: "ShowIntentionActions", + title: "Show Context Actions", + icon: , + actionPerformed: () => { + notImplemented(); + }, + }, + ], + }), + selectedText + ? { + id: CommonActionId.COPY, + title: "Copy", + icon: , + actionPerformed: () => { + const selection = editor?.getSelection(); + const selectedText = selection + ? editor?.getModel()?.getValueInRange(selection) + : ""; + if (selectedText) { + copyToClipboard(selectedText); + } + }, + } + : null, + { + id: CommonActionId.PASTE, + title: "Paste", + icon: , + actionPerformed: () => { + // Note: There doesn't seem to be a way to trigger the default paste action + // (without prompting for clipboard API), from a menu UI. + // So, selecting "Paste" menu item will ask for clipboard permission, and + // that's the same in the default Monaco context menu as well. + // The difference here is that pasting with ctrl+V works without permission + // by default, but because we are handling ctrl+V through the keymap, + // even pasting by the shortcut will require permission now. + editor?.focus(); + editor?.trigger( + "keyboard", + "editor.action.clipboardPasteAction", + {} + ); + }, + }, + createDefaultActionGroup({ + id: "Copy.Paste.Special", + title: "Copy / Paste Special", + description: "Copy / Paste Special Editor Actions", + menuPresentation: "submenu", + children: [ + { + id: CommonActionId.COPY_REFERENCE, + title: "Copy Reference", + description: + "Copy reference to selected class, method or function, or a relative path to selected file", + actionPerformed: () => { + notImplemented(); + }, + }, + { + // TODO: this action should be scoped to the entire app. + // move it when referencing actions in groups is possible + id: AppActionIds.PASTE_MULTIPLE, + title: "Paste from history", + description: "Paste from recent clipboards", + actionPerformed: () => { + notImplemented(); + }, + }, + ], + }), + "divider", + createDefaultActionGroup({ + id: "FoldingGroup", + title: "Folding", + menuPresentation: "submenu", + children: [ + { + id: editorActionIds.EXPAND_REGION, + title: "Expand", + actionPerformed: (context) => { + editorRef.current?.trigger( + source(context), + "editor.unfold", + {} + ); + }, + }, + { + id: editorActionIds.EXPAND_REGION_RECURSIVELY, + title: "Expand Recursively", + actionPerformed: (context) => { + editorRef.current?.trigger( + source(context), + "editor.unfoldRecursively", + {} + ); + }, + }, + { + id: editorActionIds.EXPAND_ALL_REGIONS, + title: "Expand All", + actionPerformed: (context) => { + editorRef.current?.trigger( + source(context), + "editor.unfoldAll", + {} + ); + }, + }, + "divider", + { + id: editorActionIds.COLLAPSE_REGION, + title: "Collapse", + actionPerformed: (context) => { + editorRef.current?.trigger(source(context), "editor.fold", {}); + }, + }, + { + id: editorActionIds.COLLAPSE_REGION_RECURSIVELY, + title: "Collapse Recursively", + actionPerformed: (context) => { + editorRef.current?.trigger( + source(context), + "editor.foldRecursively", + {} + ); + }, + }, + { + id: editorActionIds.COLLAPSE_ALL_REGIONS, + title: "Collapse All", + actionPerformed: (context) => { + editorRef.current?.trigger( + source(context), + "editor.foldAll", + {} + ); + }, + }, + "divider", + createDefaultActionGroup({ + id: editorActionIds.EXPAND_ALL_TO_LEVEL, + title: "Expand all to level", + menuPresentation: "submenu", + children: ([1, 2, 3, 4, 5] as const).map((level) => ({ + id: editorActionIds[`EXPAND_ALL_TO_LEVEL${level}`], + title: `${level}`, + actionPerformed: (context) => { + editorRef.current?.trigger( + source(context), + `editor.foldLevel${level}`, + {} + ); + }, + })), + }), + "divider", + { + id: editorActionIds.COLLAPSE_DOC_COMMENTS, + title: "Collapse Doc Comments", + actionPerformed: (context) => { + editorRef.current?.trigger( + source(context), + "editor.foldAllBlockComments", + {} + ); + }, + }, + "divider", + { + id: editorActionIds.TOGGLE_FOLDING, + title: "Toggle Folding", + actionPerformed: (context) => { + editorRef.current?.trigger( + source(context), + "editor.toggleFold", + {} + ); + }, + }, + ], + }), + createDefaultActionGroup({ + id: "EditorLangPopupMenu", + title: "EditorLangPopupMenu", + menuPresentation: "section", + children: [ + { + id: AppActionIds.GENERATE, + title: "Generate...", + actionPerformed: () => { + notImplemented(); + }, + }, + ], + }), + "divider", + { + id: editorActionIds.CompareClipboardWithSelection, + title: "Compare with Clipboard", + description: "Compare current selection with clipboard", + icon: , + actionPerformed: () => { + notImplemented(); + }, + }, + ] as const + ).filter(notNull), + }); +} + +function source(context: ActionContext): "keyboard" | "mouse" | "" { + if (!context.event) { + return ""; + } + return context.event?.type?.includes("key") ? "keyboard" : "mouse"; +} diff --git a/packages/example-app/src/Project/useProjectActions.tsx b/packages/example-app/src/Project/useProjectActions.tsx index 98b29aa1..accb2f61 100644 --- a/packages/example-app/src/Project/useProjectActions.tsx +++ b/packages/example-app/src/Project/useProjectActions.tsx @@ -30,7 +30,7 @@ export function useProjectActions(): ActionDefinition[] { const newFileAction = useRecoilValue(createFileActionState); const newDirectoryAction = useRecoilValue(createDirectoryActionState); const newElementActionGroup = createDefaultActionGroup({ - id: "NewElement", + id: projectActionIds.NewElement, title: "New...", description: "Create new class, interface, file or directory", isSearchable: true, diff --git a/packages/example-app/src/appActionIds.ts b/packages/example-app/src/appActionIds.ts new file mode 100644 index 00000000..e9911594 --- /dev/null +++ b/packages/example-app/src/appActionIds.ts @@ -0,0 +1,4 @@ +export const AppActionIds = { + PASTE_MULTIPLE: "PasteMultiple", + GENERATE: "Generate", +}; diff --git a/packages/example-app/src/exampleAppKeymap.ts b/packages/example-app/src/exampleAppKeymap.ts index 2dcd9769..7ee718bd 100644 --- a/packages/example-app/src/exampleAppKeymap.ts +++ b/packages/example-app/src/exampleAppKeymap.ts @@ -1,6 +1,9 @@ import { Keymap } from "@intellij-platform/core"; import { SearchEverywhereActionIds } from "./SearchEverywhere/SearchEverywhereActionIds"; import { VcsActionIds } from "./VersionControl/VcsActionIds"; +import { AppActionIds } from "./appActionIds"; +import { editorActionIds } from "./Editor/editorActionIds"; +import { projectActionIds } from "./Project/projectActionIds"; export const exampleAppKeymap: Keymap = { ActivateProjectWindow: [ @@ -140,7 +143,25 @@ export const exampleAppKeymap: Keymap = { }, }, ], - NewElement: [ + [AppActionIds.PASTE_MULTIPLE]: [ + { + type: "keyboard", + firstKeyStroke: { + modifiers: ["Meta", "Shift"], + code: "KeyV", + }, + }, + ], + [projectActionIds.NewElement]: [ + { + type: "keyboard", + firstKeyStroke: { + modifiers: ["Meta"], + code: "KeyN", + }, + }, + ], + [AppActionIds.GENERATE]: [ { type: "keyboard", firstKeyStroke: { @@ -149,4 +170,125 @@ export const exampleAppKeymap: Keymap = { }, }, ], + [editorActionIds.EXPAND_REGION]: [ + { + type: "keyboard", + firstKeyStroke: { + modifiers: ["Meta"], + code: "NumpadAdd", + }, + }, + { + type: "keyboard", + firstKeyStroke: { + modifiers: ["Meta"], + code: "Equal", + }, + }, + ], + [editorActionIds.EXPAND_REGION_RECURSIVELY]: [ + { + type: "keyboard", + firstKeyStroke: { + modifiers: ["Meta", "Alt"], + code: "NumpadAdd", + }, + }, + { + type: "keyboard", + firstKeyStroke: { + modifiers: ["Meta", "Alt"], + code: "Equal", + }, + }, + ], + [editorActionIds.EXPAND_ALL_REGIONS]: [ + { + type: "keyboard", + firstKeyStroke: { + modifiers: ["Meta", "Shift"], + code: "NumpadAdd", + }, + }, + { + type: "keyboard", + firstKeyStroke: { + modifiers: ["Meta", "Shift"], + code: "Equal", + }, + }, + ], + ...([1, 2, 3, 4, 5] as const).reduce((result, level) => { + result[editorActionIds[`EXPAND_ALL_TO_LEVEL${level}`]] = [ + { + type: "keyboard", + firstKeyStroke: { + modifiers: ["Alt", "Meta"], + code: "Multiply", + }, + secondKeyStroke: { + code: `Digit${level}`, + }, + }, + { + type: "keyboard", + firstKeyStroke: { + modifiers: ["Alt", "Meta"], + code: "Multiply", + }, + secondKeyStroke: { + code: `Numpad${level}`, + }, + }, + ]; + return result; + }, {} as Keymap), + [editorActionIds.COLLAPSE_REGION]: [ + { + type: "keyboard", + firstKeyStroke: { + modifiers: ["Meta"], + code: "Minus", + }, + }, + { + type: "keyboard", + firstKeyStroke: { + modifiers: ["Meta"], + code: "NumpadSubtract", + }, + }, + ], + [editorActionIds.COLLAPSE_REGION_RECURSIVELY]: [ + { + type: "keyboard", + firstKeyStroke: { + modifiers: ["Meta", "Alt"], + code: "Minus", + }, + }, + { + type: "keyboard", + firstKeyStroke: { + modifiers: ["Meta", "Alt"], + code: "NumpadSubtract", + }, + }, + ], + [editorActionIds.COLLAPSE_ALL_REGIONS]: [ + { + type: "keyboard", + firstKeyStroke: { + modifiers: ["Meta", "Shift"], + code: "Minus", + }, + }, + { + type: "keyboard", + firstKeyStroke: { + modifiers: ["Meta", "Shift"], + code: "NumpadSubtract", + }, + }, + ], }; diff --git a/packages/jui/src/ActionSystem/ActionGroup.tsx b/packages/jui/src/ActionSystem/ActionGroup.tsx index 371d10b3..f530597a 100644 --- a/packages/jui/src/ActionSystem/ActionGroup.tsx +++ b/packages/jui/src/ActionSystem/ActionGroup.tsx @@ -2,6 +2,7 @@ import { Action, ActionDefinition, } from "@intellij-platform/core/ActionSystem/Action"; +import { DividerItem } from "@intellij-platform/core/Collections"; export type ActionInResolvedGroup = Action & { parent: ResolvedActionGroup }; @@ -24,7 +25,7 @@ type ActionGroupMenuPresentation = | "submenu"; export interface MutableActionGroup extends Action { - children: Action[]; + children: Array; /** * Whether the action group is searchable. See {@link getAvailableActionsFor}. */ @@ -41,7 +42,7 @@ export interface ResolvedActionGroup extends ActionGroup { children: ActionInResolvedGroup[]; } export interface ActionGroupDefinition extends ActionDefinition { - children: ActionDefinition[]; // Should DividerItem be supported first-class here? + children: Array; /** * Defines how the action group should be represented in menus. * @default 'submenu' diff --git a/packages/jui/src/ActionSystem/ActionsProvider.tsx b/packages/jui/src/ActionSystem/ActionsProvider.tsx index e05da765..da5994ac 100644 --- a/packages/jui/src/ActionSystem/ActionsProvider.tsx +++ b/packages/jui/src/ActionSystem/ActionsProvider.tsx @@ -19,6 +19,7 @@ import { ActionDefinition, MutableAction, } from "./Action"; +import { DividerItem } from "@intellij-platform/core/Collections"; /** * Represents the properties required for the ActionsProvider component. @@ -62,9 +63,15 @@ export function ActionsProvider(props: ActionsProviderProps): JSX.Element { const keymap = useKeymap(); const actions: Action[] = []; dfsVisit( - (action: Action | null) => - action && isActionGroup(action) ? action.children : null, - (action) => actions.push(action), + (action: Action | DividerItem | null) => + action && !(action instanceof DividerItem) && isActionGroup(action) + ? action.children + : null, + (action) => { + if (action && !(action instanceof DividerItem)) { + actions.push(action); + } + }, recursivelyCreateActions(keymap, props.actions) ); @@ -119,52 +126,61 @@ function isMutableActionGroup( function recursivelyCreateActions( keymap: Keymap | null, - actionDefinitions: ActionDefinition[], + actionDefinitions: Array, parent: ActionGroup -): Array; +): Array; function recursivelyCreateActions( keymap: Keymap | null, - actionDefinitions: ActionDefinition[] -): Array; + actionDefinitions: Array +): Array; function recursivelyCreateActions( keymap: Keymap | null, - actionDefinitions: ActionDefinition[], + actionDefinitions: Array, parent?: ActionGroup -): Array { - return actionDefinitions.map((actionDefinition: ActionDefinition): Action => { - const shortcuts = - keymap?.[actionDefinition.id] ?? - (actionDefinition.useShortcutsOf - ? keymap?.[actionDefinition.useShortcutsOf] - : undefined); - const firstShortcut = shortcuts?.[0]; - const action: MutableAction | ActionInResolvedGroup = { - ...actionDefinition, - ...(isActionGroupDefinition(actionDefinition) - ? { parent: parent ?? null } - : {}), - shortcuts, - shortcut: firstShortcut ? shortcutToString(firstShortcut) : undefined, // Maybe it should be all shortcuts? - perform: (context) => { - if (!action.isDisabled) { - actionDefinition.actionPerformed( - context || { event: null, element: null } - ); - } - }, - }; - if ( - isMutableActionGroup(action) && - isActionGroupDefinition(actionDefinition) - ) { - action.children = recursivelyCreateActions( - keymap, - actionDefinition.children, - action - ); +): Array { + return actionDefinitions.map( + ( + actionDefinition: ActionDefinition | DividerItem + ): Action | DividerItem => { + if (actionDefinition instanceof DividerItem) { + return actionDefinition; + } + const shortcuts = + keymap?.[actionDefinition.id] ?? + (actionDefinition.useShortcutsOf + ? keymap?.[actionDefinition.useShortcutsOf] + : undefined); + const firstShortcut = shortcuts?.[0]; + const action: MutableAction | ActionInResolvedGroup = { + ...actionDefinition, + ...(isActionGroupDefinition(actionDefinition) + ? { parent: parent ?? null } + : {}), + shortcuts, + shortcut: firstShortcut ? shortcutToString(firstShortcut) : undefined, // Maybe it should be all shortcuts? + perform: (context) => { + if (!action.isDisabled) { + actionDefinition.actionPerformed( + context || { event: null, element: null } + ); + } + }, + }; + if ( + isMutableActionGroup(action) && + isActionGroupDefinition(actionDefinition) + ) { + action.children = recursivelyCreateActions( + keymap, + actionDefinition.children.map((child) => + child === "divider" ? new DividerItem() : child + ), + action + ); + } + return action; } - return action; - }); + ); } /** diff --git a/packages/jui/src/ActionSystem/components/ActionsMenu.tsx b/packages/jui/src/ActionSystem/components/ActionsMenu.tsx index 0a672ae3..f56aaae4 100644 --- a/packages/jui/src/ActionSystem/components/ActionsMenu.tsx +++ b/packages/jui/src/ActionSystem/components/ActionsMenu.tsx @@ -93,7 +93,7 @@ export function ActionsMenu({ type ActionAsMenuItem = Omit; export function renderActionAsMenuItem( - action: ActionAsMenuItem | ActionGroupAsMenuItem + action: ActionAsMenuItem | ActionGroupAsMenuItem | DividerItem ) { const isGroup = "children" in action; if ( @@ -118,6 +118,9 @@ export function renderActionAsMenuItem( ); } + if (action instanceof DividerItem) { + return ; + } return ( { const { show } = usePopupManager(); @@ -47,15 +49,22 @@ export const useCreateDefaultActionGroup = () => { */ + item === "divider" ? new DividerItem() : item + )} onAction={(key) => { // The need for calculating `allActions` is a consequence of the issue explained in the note above. const allActions = flatten( children.map((item) => - isActionGroupDefinition(item) ? item.children : item + item !== "divider" && isActionGroupDefinition(item) + ? item.children + : item ) ); - const action = allActions.find((action) => action.id === key); + const action = allActions.find( + (action): action is ActionDefinition => + typeof action === "object" && action.id === key + ); if (action && !action.isDisabled) { action.actionPerformed(context); } @@ -64,11 +73,15 @@ export const useCreateDefaultActionGroup = () => { autoFocus="first" > {(item) => - renderActionAsMenuItem({ - ...item, - // a consequence of the issue explained in the note above. - shortcut: getActionShortcut(item.id), - }) + renderActionAsMenuItem( + item instanceof DividerItem + ? item + : { + ...item, + // a consequence of the issue explained in the note above. + shortcut: getActionShortcut(item.id), + } + ) } } diff --git a/packages/jui/src/ActionSystem/shortcutToString.ts b/packages/jui/src/ActionSystem/shortcutToString.ts index a24a6ec0..6e6d8f87 100644 --- a/packages/jui/src/ActionSystem/shortcutToString.ts +++ b/packages/jui/src/ActionSystem/shortcutToString.ts @@ -28,6 +28,10 @@ const defaultKeyToStr: { Enter: "⏎", Quote: "'", Minus: "-", + Subtract: "-", + NumpadAdd: "+", + Multiply: "*", + NumpadMultiply: "*", Equal: "+", Backspace: "⌫", // lowercase to uppercase map diff --git a/packages/jui/src/Menu/renderMenuNodes.tsx b/packages/jui/src/Menu/renderMenuNodes.tsx index cfaa2f4c..ab8a04bc 100644 --- a/packages/jui/src/Menu/renderMenuNodes.tsx +++ b/packages/jui/src/Menu/renderMenuNodes.tsx @@ -19,7 +19,7 @@ export function renderMenuNodes( return ( <> {node.props.hasDivider && ( - + )} ( state={state} filter={filter} /> + {node.props.hasDivider && ( + + )} ); case "divider": From 0bc1cbfe04353d0d2f7b4c668550c2adc2d504f2 Mon Sep 17 00:00:00 2001 From: Alireza Date: Sun, 3 Nov 2024 22:38:22 +0100 Subject: [PATCH 04/17] improvement(ContextMenu): disable background scroll when menu is open --- .../example-app/src/Editor/FileEditor.tsx | 5 + packages/jui/package.json | 2 +- packages/jui/src/Menu/Menu.cy.tsx | 99 +++++++++++++++++++ .../jui/src/Menu/MenuOverlayFromOrigin.tsx | 39 +++++++- yarn.lock | 12 +-- 5 files changed, 148 insertions(+), 9 deletions(-) diff --git a/packages/example-app/src/Editor/FileEditor.tsx b/packages/example-app/src/Editor/FileEditor.tsx index 4e2ebac0..aaac8ea1 100644 --- a/packages/example-app/src/Editor/FileEditor.tsx +++ b/packages/example-app/src/Editor/FileEditor.tsx @@ -234,6 +234,11 @@ export const FileEditor = () => { setContextMenu(c); }); }); + monacoEditor.onDidScrollChange(() => { + // It doesn't seem easy to disable scroll on Monaco editor, so just closing the context menu on + // scroll, just like the default Monaco context menu. + setContextMenu(null); + }); enableJsx(monaco); editorRef.current = monacoEditor; monacoEditor.onDidChangeCursorPosition((e) => { diff --git a/packages/jui/package.json b/packages/jui/package.json index f8dbb0de..ec2376ee 100644 --- a/packages/jui/package.json +++ b/packages/jui/package.json @@ -113,7 +113,7 @@ "cypress-plugin-snapshots": "1.4.4", "cypress-plugin-steps": "^1.1.1", "cypress-plugin-xhr-toggle": "^1.2.1", - "cypress-real-events": "1.7.4", + "cypress-real-events": "^1.13.0", "hygen": "^6.2.11", "jest": "^29.0.3", "path-browserify": "^1.0.1", diff --git a/packages/jui/src/Menu/Menu.cy.tsx b/packages/jui/src/Menu/Menu.cy.tsx index a4e92f8c..5a39378f 100644 --- a/packages/jui/src/Menu/Menu.cy.tsx +++ b/packages/jui/src/Menu/Menu.cy.tsx @@ -887,6 +887,105 @@ describe("ContextMenu", () => { cy.wrap(onAction).should("be.calledOnce"); }); + + it("disables the scroll of the document when the menu is open", () => { + const ScrollListener = (props: { onScroll: () => void }) => { + React.useEffect(() => { + const onScroll = () => { + props.onScroll(); + }; + document.addEventListener("scroll", onScroll); + return () => { + document.removeEventListener("scroll", onScroll); + }; + }); + return null; + }; + const onScroll = cy.stub().as("onScroll"); + cy.mount( + <> + + ( + + Menu item + + )} + > + {" "} + + + ); + cy.get("#container").rightclick("top", { + scrollBehavior: false, + }); + cy.findByRole("menu"); + cy.get("#container").realMouseWheel({ + deltaY: 5, + scrollBehavior: false, // we don't want cypress to do any extra scrolling + }); + cy.wait(200); // maybe a bug in realMouseWheel, but before the wait, the next command runs before scrolling happens + cy.wrap(onScroll).should("not.be.called"); + }); + + it("disables the scroll when the menu is open", () => { + const onScroll = cy.stub().as("onScroll"); + cy.mount( + ( + + Menu item + + )} + > +
+ + ); + cy.get("#container").rightclick("left", { + scrollBehavior: false, + }); + cy.findByRole("menu"); + cy.get("#container").realMouseWheel({ + deltaY: 5, + scrollBehavior: false, // we don't want cypress to do any extra scrolling + }); + cy.wait(200); // maybe a bug in realMouseWheel, but before the wait, the next command runs before scrolling happens + cy.wrap(onScroll).should("not.be.called"); + }); + + it("disables the scroll of the scrollable parent when the menu is open", () => { + const onScroll = cy.stub().as("onScroll"); + cy.mount( + ( + + Menu item + + )} + > +
+
+
+ + ); + cy.get("#container").rightclick("left", { + scrollBehavior: false, + }); + cy.findByRole("menu"); + cy.get("#container").realMouseWheel({ + deltaY: 5, + scrollBehavior: false, // we don't want cypress to do any extra scrolling + }); + cy.wait(200); // maybe a bug in realMouseWheel, but before the wait, the next command runs before scrolling happens + cy.wrap(onScroll).should("not.be.called"); + }); }); function matchImageSnapshot(snapshotsName: string) { diff --git a/packages/jui/src/Menu/MenuOverlayFromOrigin.tsx b/packages/jui/src/Menu/MenuOverlayFromOrigin.tsx index 3edd35f5..2bb992d6 100644 --- a/packages/jui/src/Menu/MenuOverlayFromOrigin.tsx +++ b/packages/jui/src/Menu/MenuOverlayFromOrigin.tsx @@ -1,10 +1,11 @@ -import React, { ForwardedRef } from "react"; +import React, { ForwardedRef, useEffect, useMemo } from "react"; import { MenuOverlay, MenuOverlayProps, } from "@intellij-platform/core/Menu/MenuOverlay"; import { useOverlayPositionFromOrigin } from "@intellij-platform/core/Menu/useOverlayPositionFromOrigin"; -import { useObjectRef } from "@react-aria/utils"; +import { getScrollParent, isScrollable, useObjectRef } from "@react-aria/utils"; +import { createGlobalStyle } from "styled-components"; interface MenuOverlayFromOriginProps extends Pick { @@ -24,11 +25,24 @@ interface MenuOverlayFromOriginProps * See {@link MouseEvent.clientX} */ clientY: number; + + /** + * Origin's target element. + * Used to find the scrollable parent and disable scrolling while + * the overlay is rendered. + */ + target?: EventTarget | HTMLElement | null; } | undefined; children: React.ReactNode; } +const DisableScrollStyles = createGlobalStyle` + .disable-scroll { + overflow: hidden !important; + } +`; + /** * Menu overlay position based on an origin point on the screen. * Useful when the menu is opened by a pointer event. @@ -44,8 +58,29 @@ export const MenuOverlayFromOrigin = React.forwardRef( origin, containerPadding: { x: 0, y: 4 }, }); + + const scrollParent: null | Element = useMemo(() => { + if (!(origin?.target instanceof HTMLElement)) { + return null; + } + return isScrollable(origin.target) + ? origin.target + : getScrollParent(origin.target); + }, [origin?.target]); + + useEffect(() => { + // Known issue: closing contextmenu causes a jump in scroll (see project view in example app) + // It's an issue that existed before the change to disable scroll when the contextmenu is open, + // so it should not have anythign to do with changing overflow hidden when the menu is open. + scrollParent?.classList?.add("disable-scroll"); + return () => { + scrollParent?.classList?.remove("disable-scroll"); + }; + }, []); + return ( <> + {Boolean(origin) && ( Date: Mon, 18 Nov 2024 21:27:58 +0100 Subject: [PATCH 05/17] fix(ActionsMenu): fix props menu props not passed to Menu --- packages/jui/src/ActionSystem/components/ActionsMenu.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/jui/src/ActionSystem/components/ActionsMenu.tsx b/packages/jui/src/ActionSystem/components/ActionsMenu.tsx index f56aaae4..71879119 100644 --- a/packages/jui/src/ActionSystem/components/ActionsMenu.tsx +++ b/packages/jui/src/ActionSystem/components/ActionsMenu.tsx @@ -55,7 +55,7 @@ export type ActionMenuProps = { export function ActionsMenu({ actions, actionContext, - children = (actionMenuProps) => , + children = (actionMenuProps) => , ...otherProps }: ActionMenuProps) { const allActions = getAllActions(actions); @@ -66,6 +66,7 @@ export function ActionsMenu({ return ( <> {children({ + ...otherProps, onAction: (key) => { const action = allActions.find(({ id }) => id === key); if (action && isAction(action)) { From b5e830870dcb9dd0dd7e8316c5a08973e4bf63cc Mon Sep 17 00:00:00 2001 From: Alireza Date: Wed, 20 Nov 2024 23:03:18 +0100 Subject: [PATCH 06/17] fix(Input): don't stop propagation of keyboard events --- packages/jui/src/InputField/Input.cy.tsx | 18 ++++++++++++++++++ packages/jui/src/InputField/Input.tsx | 4 ---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/jui/src/InputField/Input.cy.tsx b/packages/jui/src/InputField/Input.cy.tsx index 9bdd62ea..989e5151 100644 --- a/packages/jui/src/InputField/Input.cy.tsx +++ b/packages/jui/src/InputField/Input.cy.tsx @@ -46,10 +46,28 @@ describe("Input", () => { cy.mount(); cy.wrap(ref).its("current").should("be.instanceOf", HTMLDivElement); }); + it("forwards ref to the input element", () => { const ref = React.createRef(); cy.mount(); cy.wrap(ref).its("current").should("be.instanceOf", HTMLInputElement); }); + + it("doesn't stop propagation of keyboard events", () => { + const onKeyDown = cy.stub(); + const onKeyUp = cy.stub(); + const onParentKeyDown = cy.stub(); + const onParentKeyUp = cy.stub(); + cy.mount( +
+ +
+ ); + cy.get("input").focus().type("a"); + cy.wrap(onKeyDown).should("be.calledOnce"); + cy.wrap(onKeyUp).should("be.calledOnce"); + cy.wrap(onParentKeyDown).should("be.calledOnce"); + cy.wrap(onParentKeyUp).should("be.calledOnce"); + }); }); }); diff --git a/packages/jui/src/InputField/Input.tsx b/packages/jui/src/InputField/Input.tsx index 779dfd07..28c64c99 100644 --- a/packages/jui/src/InputField/Input.tsx +++ b/packages/jui/src/InputField/Input.tsx @@ -146,8 +146,6 @@ export const Input = React.forwardRef(function Input( style, className, inputRef: inputRefProp, - onKeyDown, - onKeyUp, onFocus, onBlur, autoFocus, @@ -164,8 +162,6 @@ export const Input = React.forwardRef(function Input( autoFocus, onFocus, onBlur, - onKeyDown, - onKeyUp, } as FocusableOptions, inputRef ); From 599372dd36236034bbab7eb2e32f0dca3d106633 Mon Sep 17 00:00:00 2001 From: Alireza Date: Wed, 20 Nov 2024 23:08:32 +0100 Subject: [PATCH 07/17] feat(example-app): add path autocompletion in clone action --- packages/example-app/package.json | 2 + .../SearchEverywherePopup.tsx | 2 +- .../BranchesView/BranchesTree.tsx | 2 +- .../VersionControl/actions/PathInputField.tsx | 243 ++++++++++++++++++ .../actions/getInputCursorOffset.ts | 28 ++ .../VersionControl/actions/gitCloneAction.tsx | 41 +-- packages/jui/cypress/e2e/vcs/clone.cy.ts | 32 ++- .../support/example-app/initializers.ts | 13 + .../jui/src/ActionSystem/CommonActionIds.ts | 1 + .../jui/src/ActionSystem/defaultKeymap.tsx | 9 + .../Collections/useCollectionSearchInput.ts | 14 +- packages/jui/src/Dropdown/ComboBox.tsx | 2 +- packages/jui/src/Dropdown/Dropdown.tsx | 2 +- packages/jui/src/Dropdown/ListBox.tsx | 32 +++ .../jui/src/Dropdown/StatelessListBox.tsx | 45 +++- packages/jui/src/Dropdown/index.ts | 1 + packages/jui/src/List/story-helpers.tsx | 2 +- 17 files changed, 414 insertions(+), 57 deletions(-) create mode 100644 packages/example-app/src/VersionControl/actions/PathInputField.tsx create mode 100644 packages/example-app/src/VersionControl/actions/getInputCursorOffset.ts create mode 100644 packages/jui/src/Dropdown/ListBox.tsx diff --git a/packages/example-app/package.json b/packages/example-app/package.json index 91d2784e..3360064a 100644 --- a/packages/example-app/package.json +++ b/packages/example-app/package.json @@ -14,6 +14,8 @@ "//alias": "Nested imports from @intellij-platform/core/* are not a part of public API and are listed individually here, until it's clearer if they should be a part of the public API", "alias": { "caf": "caf/dist/esm/index.mjs", + "@intellij-platform/core/utils/usePrevious": "@intellij-platform/core/src/utils/usePrevious", + "@intellij-platform/core/Menu/StyledMenu": "@intellij-platform/core/src/Menu/StyledMenu", "@intellij-platform/core/utils/tree-utils": "@intellij-platform/core/src/utils/tree-utils", "@intellij-platform/core/utils/array-utils": "@intellij-platform/core/src/utils/array-utils", "@intellij-platform/core/utils/useEventCallback": "@intellij-platform/core/src/utils/useEventCallback", diff --git a/packages/example-app/src/SearchEverywhere/SearchEverywherePopup.tsx b/packages/example-app/src/SearchEverywhere/SearchEverywherePopup.tsx index be7f1819..ae29d3f3 100644 --- a/packages/example-app/src/SearchEverywhere/SearchEverywherePopup.tsx +++ b/packages/example-app/src/SearchEverywhere/SearchEverywherePopup.tsx @@ -320,7 +320,7 @@ export function SearchEverywherePopup() { const { collectionSearchInputProps } = useCollectionSearchInput({ collectionRef, onAction, - selectionManager: selectionManagerRef.current, + selectionManagerRef, }); const tips = useTips(); diff --git a/packages/example-app/src/VersionControl/VersionControlToolWindow/BranchesView/BranchesTree.tsx b/packages/example-app/src/VersionControl/VersionControlToolWindow/BranchesView/BranchesTree.tsx index d3804709..a81acda3 100644 --- a/packages/example-app/src/VersionControl/VersionControlToolWindow/BranchesView/BranchesTree.tsx +++ b/packages/example-app/src/VersionControl/VersionControlToolWindow/BranchesView/BranchesTree.tsx @@ -67,7 +67,7 @@ export function BranchesTree({ tabKey }: { tabKey: string }) { const { collectionSearchInputProps } = useCollectionSearchInput({ collectionRef: ref, - selectionManager: selectionManagerRef.current, + selectionManagerRef, onAction, }); /** diff --git a/packages/example-app/src/VersionControl/actions/PathInputField.tsx b/packages/example-app/src/VersionControl/actions/PathInputField.tsx new file mode 100644 index 00000000..2c576f95 --- /dev/null +++ b/packages/example-app/src/VersionControl/actions/PathInputField.tsx @@ -0,0 +1,243 @@ +import path from "path"; +import React, { ComponentProps, useEffect, useRef, useState } from "react"; +import { mergeProps, mergeRefs } from "@react-aria/utils"; +import { useControlledState } from "@react-stately/utils"; +import { + ActionsProvider, + ActionTooltip, + AutoHoverPlatformIcon, + CommonActionId, + InputField, + Item, + ItemLayout, + ListBox, + MenuOverlayFromOrigin, + PlatformIcon, + styled, + TooltipTrigger, + useCollectionSearchInput, + WINDOW_SHADOW, +} from "@intellij-platform/core"; +import { MENU_VERTICAL_PADDING } from "@intellij-platform/core/Menu/StyledMenu"; +import { TreeSelectionManager } from "@intellij-platform/core/Tree/TreeSelectionManager"; +import { usePrevious } from "@intellij-platform/core/utils/usePrevious"; +import { notImplemented } from "../../Project/notImplemented"; +import { DIR_ICON } from "../../file-utils"; +import { fs } from "../../fs/fs"; +import { getInputCursorOffset } from "./getInputCursorOffset"; + +const StyledPopover = styled.div` + box-sizing: border-box; + // not checked if there should be a better substitute for * in the following colors. Maybe "Component"? + background-color: ${({ theme }) => theme.color("*.background")}; + color: ${({ theme }) => theme.color("*.foreground")}; + outline: none; // Focus will be reflected in header. No need for outline or any other focus style on the container + ${WINDOW_SHADOW}; // FIXME: OS-dependant style? +`; +const ListBoxStyledAsMenu: typeof ListBox /* https://github.com/styled-components/styled-components/issues/1803#issuecomment-2177525252 */ = styled( + ListBox +)` + min-width: 120px; + margin: ${MENU_VERTICAL_PADDING}px 0; +`; + +async function getChildDirectories(searchDirectory: string) { + return fs.promises.readdir(searchDirectory).then(async (fileNames) => { + const childItems = await Promise.all( + fileNames.map(async (fileName) => { + const fullPath = path.join(searchDirectory, fileName); + return { + fullPath, + stat: await fs.promises.stat(fullPath), + }; + }) + ); + return childItems + .filter(({ stat }) => stat.isDirectory()) + .map(({ fullPath }) => fullPath); + }); +} + +export function PathInputField({ + value: valueProp, + onChange: onChangeProp, + defaultValue: defaultValueProp, + ...props +}: Omit, "value" | "defaultValue"> & { + value?: string; + defaultValue?: string; +}) { + const [value, setValue] = useControlledState( + valueProp, + defaultValueProp ?? "", + onChangeProp + ); + const [isAutocompleteVisible, setAutocompleteVisible] = useState(false); + const inputRef = useRef(null); + + const collectionRef = useRef(null); + const selectionManagerRef = useRef(null); + const onAutocompleteSuggestionSelected = (key: React.Key) => { + setValue(path.join(`${key}`, path.sep)); // TODO: handle completion with tab + }; + const { collectionSearchInputProps } = useCollectionSearchInput({ + collectionRef, + onAction: onAutocompleteSuggestionSelected, + selectionManagerRef, + }); + // Path search query is kept in a separate state because it needs to be updated when autocomplete action triggers. + // Otherwise, it could be a computed value based on `isAutocompleteVisible` and `directory` + const [pathSearchQuery, setPathSearchQuery] = useState(""); + const [pathSuggestions, setPathSuggestions] = useState< + Array<{ fullPath: string; textValue: string }> + >([]); + const updatePathAutocompletion = () => { + setMenuOrigin({ + clientX: + (inputClientRect?.x ?? 0) + getInputCursorOffset(inputRef.current!), + clientY: (inputClientRect?.y ?? 0) + (inputClientRect?.height ?? 0) + 2, + }); + setPathSearchQuery( + value.slice(0, inputRef.current?.selectionStart ?? undefined) + ); + }; + useEffect(() => { + if (isAutocompleteVisible) { + updatePathAutocompletion(); + } + }, [isAutocompleteVisible, value]); + + const wasAutocompleteVisible = usePrevious(isAutocompleteVisible); + + useEffect(() => { + const pathParts = pathSearchQuery.split(path.sep); + const searchDirectory = pathParts.slice(0, -1).join(path.sep) || "/"; + const query = pathParts.length > 1 ? pathParts.slice(-1)[0] : ""; + let canceled = false; + getChildDirectories(searchDirectory).then((suggestions) => { + if (canceled) { + return; + } + // TODO: handle "No suggestions" + const matchedSuggestions = suggestions + .map((fullPath) => { + const dirname = path.basename(fullPath); + return { fullPath, textValue: dirname }; + }) + .filter( + ({ textValue }) => + !query || textValue.toLowerCase().includes(query.toLowerCase()) + ); + setPathSuggestions(matchedSuggestions); + // TODO: set "Use {0} to keep tail of the path" tip if caret position is not at the end. + if ( + matchedSuggestions.length > 0 && + /* Note that wasAutocompleteVisible is a captured closure and it's intentionally not in effect's dependencies */ + !wasAutocompleteVisible && + inputRef.current?.value.indexOf(pathSearchQuery) === 0 + ) { + inputRef.current.setSelectionRange( + pathSearchQuery.length, + inputRef.current.value.length, + "backward" + ); + } + }); + return () => { + canceled = true; + }; + }, [pathSearchQuery]); + + const [menuOrigin, setMenuOrigin] = useState({ clientX: 0, clientY: 0 }); + + const inputClientRect = inputRef.current?.getBoundingClientRect(); + + return ( + <> + { + updatePathAutocompletion(); + setAutocompleteVisible(true); + }, + }, + ]} + > + {({ shortcutHandlerProps }) => ( + { + if (e.key === "Escape") { + setAutocompleteVisible(false); + } + }, + onBlur: () => { + setAutocompleteVisible(false); + }, + } + )} + addonAfter={ + <> + {props.addonAfter} + } + > + {(props) => ( + + )} + + + } + /> + )} + + {isAutocompleteVisible && ( + { + setAutocompleteVisible(false); + }} + origin={menuOrigin} + > + + + {({ fullPath }) => ( + + + + {path.basename(fullPath)} + + + )} + + + + )} + + ); +} diff --git a/packages/example-app/src/VersionControl/actions/getInputCursorOffset.ts b/packages/example-app/src/VersionControl/actions/getInputCursorOffset.ts new file mode 100644 index 00000000..672c9112 --- /dev/null +++ b/packages/example-app/src/VersionControl/actions/getInputCursorOffset.ts @@ -0,0 +1,28 @@ +export function getInputCursorOffset(input: HTMLInputElement): number { + const caretIndex = input.selectionStart ?? input.value.length; + const inputStyle = window.getComputedStyle(input); + + // Create and style a hidden span to mirror the input + const hiddenSpan = document.createElement("span"); + hiddenSpan.style.position = "absolute"; + hiddenSpan.style.visibility = "hidden"; + hiddenSpan.style.whiteSpace = "pre-wrap"; // Preserve spaces and line breaks + hiddenSpan.style.font = inputStyle.font; + hiddenSpan.style.fontSize = inputStyle.fontSize; + hiddenSpan.style.fontWeight = inputStyle.fontWeight; + hiddenSpan.style.fontFamily = inputStyle.fontFamily; + hiddenSpan.style.letterSpacing = inputStyle.letterSpacing; + hiddenSpan.style.border = inputStyle.border; + hiddenSpan.style.paddingLeft = inputStyle.paddingLeft; + + // Set span content to text before the caret + hiddenSpan.textContent = input.value.substring(0, caretIndex); + document.body.appendChild(hiddenSpan); + + // Measure the width of the hidden span to get the caret offset + const caretOffset = hiddenSpan.offsetWidth; + + // Clean up the hidden span + document.body.removeChild(hiddenSpan); + return caretOffset; +} diff --git a/packages/example-app/src/VersionControl/actions/gitCloneAction.tsx b/packages/example-app/src/VersionControl/actions/gitCloneAction.tsx index 527495e9..242a9339 100644 --- a/packages/example-app/src/VersionControl/actions/gitCloneAction.tsx +++ b/packages/example-app/src/VersionControl/actions/gitCloneAction.tsx @@ -1,30 +1,26 @@ -import { createAction } from "../../createAction"; -import { windowManagerRefState } from "../../Project/project.state"; -import { VcsActionIds } from "../VcsActionIds"; +import path from "path"; +import React, { useRef, useState } from "react"; +import { useRecoilState, useRecoilValue, useSetRecoilState } from "recoil"; import { - ActionTooltip, - AutoHoverPlatformIcon, Button, ComboBox, - InputField, Item, LabeledControlsAlignmentProvider, ModalWindow, styled, - TooltipTrigger, WindowLayout, } from "@intellij-platform/core"; -import React, { useRef, useState } from "react"; -import { notImplemented } from "../../Project/notImplemented"; -import { useRecoilState, useRecoilValue, useSetRecoilState } from "recoil"; +import { windowManagerRefState } from "../../Project/project.state"; +import { fs } from "../../fs/fs"; +import { stat } from "../../fs/fs-utils"; +import { createAction } from "../../createAction"; +import { VcsActionIds } from "../VcsActionIds"; import { cloneParentDirState, gitVisitedUrlsState, } from "../gitRememberedInputs.state"; -import path from "path"; import { useClone } from "../useClone"; -import { fs } from "../../fs/fs"; -import { stat } from "../../fs/fs-utils"; +import { PathInputField } from "./PathInputField"; export const gitCloneActionSelector = createAction({ id: VcsActionIds.GIT_CLONE, @@ -130,7 +126,7 @@ function GitCloneWindow({ } return ( - + {({ url }) => {url}} - } - > - {(props) => ( - - )} - - } onChange={(value) => { setDirectory(value); setAutoFillDirectory(false); diff --git a/packages/jui/cypress/e2e/vcs/clone.cy.ts b/packages/jui/cypress/e2e/vcs/clone.cy.ts index 3b480f13..6bd41d24 100644 --- a/packages/jui/cypress/e2e/vcs/clone.cy.ts +++ b/packages/jui/cypress/e2e/vcs/clone.cy.ts @@ -1,4 +1,4 @@ -import { persistedGitSettings } from "../../support/example-app"; +import { cd, dir, persistedGitSettings } from "../../support/example-app"; it("clones a git repo", () => { cy.initialization( @@ -10,7 +10,8 @@ it("clones a git repo", () => { ], cloneParentDir: "/workspace", }, - }) + }), + cd("/", dir("/Users/ali/workspace"), dir("/Users/alireza")) ); cy.searchAndInvokeAction("clone"); cy.findByLabelText(/Directory/i).should("have.value", "/workspace"); @@ -19,8 +20,31 @@ it("clones a git repo", () => { .should("be.focused") .type("{downArrow}"); cy.findByRole("option", { name: /example-branches/ }).click(); - cy.findByRole("combobox", { name: /Url/i }) - .should("have.value", "https://github.com/thurwitz/example-branches.git") + cy.findByRole("combobox", { name: /Url/i }).should( + "have.value", + "https://github.com/thurwitz/example-branches.git" + ); + cy.findByLabelText(/Directory/i) + .should("have.value", "/workspace/example-branches") // clone directory auto set when not changed manually + .focus() + .type("{moveToStart}{rightArrow}") + .realPress(["Control", " "]); // invoke autocompletion + cy.findByLabelText(/Directory/i) + .its("[0].selectionStart") + .should("eq", 1); + cy.findByLabelText(/Directory/i) + .its("[0].selectionEnd") + .should("eq", 1 + "workspace/example-branches".length); + cy.realPress("ArrowDown").realPress("Enter"); + // There is currently an issue with autocomplete popover getting aria-hidden + // cy.findByRole("option", { name: "alireza" }) + cy.contains("alireza") // FIXME: switch to the line above. + .click() + .realType("example-branches"); + cy.findByLabelText(/Directory/i) + .should("have.value", "/Users/alireza/example-branches") + .type("{selectAll}/workspace/example-branches") .type("{enter}"); + cy.contains("Cloning"); cy.findTreeNodeInProjectView("example-branches"); }); diff --git a/packages/jui/cypress/support/example-app/initializers.ts b/packages/jui/cypress/support/example-app/initializers.ts index f9eb1e91..f8b8f490 100644 --- a/packages/jui/cypress/support/example-app/initializers.ts +++ b/packages/jui/cypress/support/example-app/initializers.ts @@ -189,6 +189,19 @@ export function dir(dirname: string, changes: Change[] = []): Change { }; } +/** + * Runs the passed changes within the context of the provided directory + * @param dir directory to run changes in + * @param changes changes to run within the context of the provided directory. + */ +export function cd(dir: string, ...changes: Change[]): Change { + return async (args, context) => { + for (const change of changes) { + await change(args, { ...context, dir }); + } + }; +} + /** * Creates an initializer which writes a file on a path specified by {@link filename} * @param filename path of the file to write, relative to the project directory. diff --git a/packages/jui/src/ActionSystem/CommonActionIds.ts b/packages/jui/src/ActionSystem/CommonActionIds.ts index f57b1084..1a19d3bb 100644 --- a/packages/jui/src/ActionSystem/CommonActionIds.ts +++ b/packages/jui/src/ActionSystem/CommonActionIds.ts @@ -14,4 +14,5 @@ export const CommonActionId = { CUT: "$Cut", PASTE: "$Paste", DELETE: "$Delete", + CODE_COMPLETION: "CodeCompletion", }; diff --git a/packages/jui/src/ActionSystem/defaultKeymap.tsx b/packages/jui/src/ActionSystem/defaultKeymap.tsx index 09e823a4..2e35cb74 100644 --- a/packages/jui/src/ActionSystem/defaultKeymap.tsx +++ b/packages/jui/src/ActionSystem/defaultKeymap.tsx @@ -270,4 +270,13 @@ export const defaultKeymap: Keymap = { }, }, ], + [CommonActionId.CODE_COMPLETION]: [ + { + type: "keyboard", + firstKeyStroke: { + modifiers: ["Control"], + code: "Space", + }, + }, + ], }; diff --git a/packages/jui/src/Collections/useCollectionSearchInput.ts b/packages/jui/src/Collections/useCollectionSearchInput.ts index fe4a0dcc..1f0e4597 100644 --- a/packages/jui/src/Collections/useCollectionSearchInput.ts +++ b/packages/jui/src/Collections/useCollectionSearchInput.ts @@ -27,7 +27,7 @@ import { DOMAttributes } from "@react-types/shared"; */ export const useCollectionSearchInput = ({ collectionRef, - selectionManager, + selectionManagerRef, onAction, }: { /** @@ -39,7 +39,7 @@ export const useCollectionSearchInput = ({ * {@link CollectionRefProps.selectionManagerRef} can be used on collection components that implement * `useCollectionRef`, to get a hold of selection manager, from outside. */ - selectionManager: SelectionManager | null | undefined; + selectionManagerRef: RefObject | null | undefined; /** * onAction callback passed to the collection component. It's needed since some upgrade of @react-aria/interactions, * since a check is added to not have keyup events on outside elements trigger onPress. That's to prevent scenarios @@ -64,13 +64,17 @@ export const useCollectionSearchInput = ({ } else if ( event.type === "keydown" && event.key === "Enter" && - selectionManager?.focusedKey != null + selectionManagerRef?.current?.focusedKey != null ) { + event.preventDefault(); // in forms, pressing Enter on input submits the form event.currentTarget.addEventListener( "keyup", (event: KeyboardEvent) => { - if (event.key === "Enter" && selectionManager?.focusedKey != null) { - onAction?.(selectionManager?.focusedKey); + if ( + event.key === "Enter" && + selectionManagerRef?.current?.focusedKey != null + ) { + onAction?.(selectionManagerRef?.current?.focusedKey); } }, { once: true, capture: true } diff --git a/packages/jui/src/Dropdown/ComboBox.tsx b/packages/jui/src/Dropdown/ComboBox.tsx index c33309d1..8db030cd 100644 --- a/packages/jui/src/Dropdown/ComboBox.tsx +++ b/packages/jui/src/Dropdown/ComboBox.tsx @@ -274,7 +274,7 @@ export const ComboBox = forwardRef( {...listBoxProps} ref={listBoxRef} state={state} - minWidth={minWidth} + style={{ minWidth }} /> )} diff --git a/packages/jui/src/Dropdown/Dropdown.tsx b/packages/jui/src/Dropdown/Dropdown.tsx index f3f55ef1..c934d4a3 100644 --- a/packages/jui/src/Dropdown/Dropdown.tsx +++ b/packages/jui/src/Dropdown/Dropdown.tsx @@ -108,7 +108,7 @@ export const Dropdown = forwardRef( )} diff --git a/packages/jui/src/Dropdown/ListBox.tsx b/packages/jui/src/Dropdown/ListBox.tsx new file mode 100644 index 00000000..d4a1caeb --- /dev/null +++ b/packages/jui/src/Dropdown/ListBox.tsx @@ -0,0 +1,32 @@ +import React, { ForwardedRef } from "react"; +import { AriaListBoxOptions, AriaListBoxProps } from "@react-aria/listbox"; +import { useListState } from "@react-stately/list"; +import { StatelessListBox } from "@intellij-platform/core/Dropdown/StatelessListBox"; +import { + CollectionRefProps, + useCollectionRef, +} from "@intellij-platform/core/Collections/useCollectionRef"; + +export interface ListBoxProps + extends AriaListBoxProps, + CollectionRefProps, + Pick< + AriaListBoxOptions, + "shouldFocusOnHover" | "shouldUseVirtualFocus" + > { + minWidth?: number; +} + +export const ListBox = React.forwardRef(function ListBox( + { minWidth, ...props }: ListBoxProps, + forwardedRef: ForwardedRef +) { + const state = useListState({ + selectionBehavior: "toggle", + selectionMode: "single", + ...props, + }); + useCollectionRef(props, state); + + return ; +}); diff --git a/packages/jui/src/Dropdown/StatelessListBox.tsx b/packages/jui/src/Dropdown/StatelessListBox.tsx index 37bb4dd7..3b260b08 100644 --- a/packages/jui/src/Dropdown/StatelessListBox.tsx +++ b/packages/jui/src/Dropdown/StatelessListBox.tsx @@ -1,29 +1,32 @@ -import React, { ForwardedRef, useRef } from "react"; +import React, { CSSProperties, ForwardedRef } from "react"; import { AriaListBoxOptions, useListBox, useListBoxSection, useOption, } from "@react-aria/listbox"; +import { useObjectRef } from "@react-aria/utils"; import { ListState } from "@react-stately/list"; import { Node } from "@react-types/shared"; +import { ItemStateContext } from "@intellij-platform/core/Collections"; import { StyledListItem } from "@intellij-platform/core/List/StyledListItem"; import { StyledList } from "@intellij-platform/core/List/StyledList"; import { StyledVerticalSeparator } from "@intellij-platform/core/StyledSeparator"; import { styled } from "@intellij-platform/core/styled"; -import { useObjectRef } from "@react-aria/utils"; export const StatelessListBox = React.forwardRef(function StatelessListBox< T extends object >( { state, - minWidth, + style, + className, ...props }: AriaListBoxOptions & { state: ListState; - minWidth?: number; + style?: CSSProperties; + className?: string; }, forwardedRef: ForwardedRef ) { @@ -33,7 +36,12 @@ export const StatelessListBox = React.forwardRef(function StatelessListBox< return ( <>
{props.label}
- + {[...state.collection].map((item) => item.type === "section" ? ( @@ -55,16 +63,27 @@ function Option({ const ref = React.useRef(null); const { optionProps } = useOption({ key: item.key }, state, ref); + const isDisabled = state.disabledKeys.has(item.key); + const isSelected = state.selectionManager.focusedKey === item.key; return ( - - {item.rendered} - + + {item.rendered} + + ); } diff --git a/packages/jui/src/Dropdown/index.ts b/packages/jui/src/Dropdown/index.ts index d0fe0987..b891da25 100644 --- a/packages/jui/src/Dropdown/index.ts +++ b/packages/jui/src/Dropdown/index.ts @@ -1,3 +1,4 @@ export type { DropdownProps } from "./Dropdown"; export { Dropdown } from "./Dropdown"; export { ComboBox, type ComboBoxProps } from "./ComboBox"; +export { ListBox, type ListBoxProps } from "./ListBox"; diff --git a/packages/jui/src/List/story-helpers.tsx b/packages/jui/src/List/story-helpers.tsx index 96079103..f7d5217e 100644 --- a/packages/jui/src/List/story-helpers.tsx +++ b/packages/jui/src/List/story-helpers.tsx @@ -61,7 +61,7 @@ export const commonListStories = { const { collectionSearchInputProps } = useCollectionSearchInput({ collectionRef: listRef, onAction: props.onAction, - selectionManager: selectionManagerRef.current, + selectionManagerRef, }); return ( From 3de14ebed0203a487f4a42c4d71dea88ee96382d Mon Sep 17 00:00:00 2001 From: Alireza Date: Thu, 5 Dec 2024 22:34:47 +0100 Subject: [PATCH 08/17] fix(ModalWindow): allow focus to go to overlays opened from within the modal window --- .../integration/modal-window_menu.cy.tsx | 50 ++++++++++++++++++- packages/jui/src/ModalWindow/ModalWindow.tsx | 10 +++- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/packages/jui/cypress/integration/modal-window_menu.cy.tsx b/packages/jui/cypress/integration/modal-window_menu.cy.tsx index 29c98264..52a7d0bb 100644 --- a/packages/jui/cypress/integration/modal-window_menu.cy.tsx +++ b/packages/jui/cypress/integration/modal-window_menu.cy.tsx @@ -1,15 +1,61 @@ -import React, { useState } from "react"; +import React, { useRef, useState } from "react"; import { - IconButton, FocusScope, + IconButton, Item, Menu, + MenuOverlayFromOrigin, MenuTrigger, ModalWindow, PlatformIcon, WindowLayout, } from "@intellij-platform/core"; +it("allows the focus to go out of modal window and into nested overlays such as menu", () => { + const Example = () => { + const [showMenu, setShowMenu] = useState(false); + const inputRef = useRef(null); + return ( + + + setShowMenu(e.currentTarget.value !== "")} + /> + {showMenu && ( + { + setShowMenu(false); + }} + > +
+ +
+ + )} +
+ } + /> + + ); + }; + cy.mount(); + cy.get("input").type("a"); + cy.focused().should("contain.text", "focused element inside nested overlay"); + cy.realPress("Enter"); + cy.get("input").should("be.focused"); +}); + it("moves focus to the modal window, when opened by a menu item action", () => { cy.mount(); cy.findByRole("button").click(); diff --git a/packages/jui/src/ModalWindow/ModalWindow.tsx b/packages/jui/src/ModalWindow/ModalWindow.tsx index ef6e7848..a16a57cb 100644 --- a/packages/jui/src/ModalWindow/ModalWindow.tsx +++ b/packages/jui/src/ModalWindow/ModalWindow.tsx @@ -173,7 +173,15 @@ function useFocusContainmentFix() { if ( !probablyFocusedElement || (probablyFocusedElement instanceof Element && - !e.currentTarget.contains(probablyFocusedElement)) + !e.currentTarget.contains(probablyFocusedElement) && + // The following condition is added to exclude cases where the focus + // is going to an overlay that is opened from within the modal window. + // The condition is suboptimal as there is no check on whether the + // overlay is a logical child of the modal or not. + // However, it seems justified since the entire hook is a temporary hack, + // and also it doesn't seem likely for an overlay outside the modal + // to grab the focus. + !probablyFocusedElement.closest("[data-overlay-root]")) ) { const elementToFocus = lastFocusedElementRef.current; if (elementToFocus) { From 8215b39752567e92d95de428247112d9a646a239 Mon Sep 17 00:00:00 2001 From: Alireza Date: Sat, 7 Dec 2024 20:40:10 +0100 Subject: [PATCH 09/17] refactor: replace custom useSelectableCollection with a wrapper around the original one One of the addressed issues is https://github.com/adobe/react-spectrum/issues/1842 which is [fixed](https://github.com/adobe/react-spectrum/pull/1843) already. The other issue is https://github.com/adobe/react-spectrum/issues/4391 which is possible to work around in a wrapper. --- .../src/selection/useSelectableCollection.ts | 436 ++---------------- 1 file changed, 32 insertions(+), 404 deletions(-) diff --git a/packages/jui/src/selection/useSelectableCollection.ts b/packages/jui/src/selection/useSelectableCollection.ts index 05ac79d1..7201e18d 100644 --- a/packages/jui/src/selection/useSelectableCollection.ts +++ b/packages/jui/src/selection/useSelectableCollection.ts @@ -1,414 +1,42 @@ -/* - * Copyright 2020 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ - -import React, { - FocusEvent, - HTMLAttributes, - Key, - KeyboardEvent, - RefObject, - useEffect, -} from "react"; -import { focusSafely, getFocusableTreeWalker } from "@react-aria/focus"; -import { FocusStrategy, KeyboardDelegate } from "@react-types/shared"; -import { mergeProps } from "@react-aria/utils"; -import { MultipleSelectionManager } from "@react-stately/selection"; -import { useLocale } from "@react-aria/i18n"; -import { useTypeSelect } from "@react-aria/selection"; -import { isCtrlKeyPressed } from "../utils/keyboard-utils"; - -interface SelectableCollectionOptions { - /** - * An interface for reading and updating multiple selection state. - */ - selectionManager: MultipleSelectionManager; - /** - * A delegate object that implements behavior for keyboard focus movement. - */ - keyboardDelegate: KeyboardDelegate; - /** - * The ref attached to the element representing the collection. - */ - ref: RefObject; - /** - * Whether the collection or one of its items should be automatically focused upon render. - * @default false - */ - autoFocus?: boolean | FocusStrategy; - /** - * Whether focus should wrap around when the end/start is reached. - * @default false - */ - shouldFocusWrap?: boolean; - /** - * Whether the collection allows empty selection. - * @default false - */ - disallowEmptySelection?: boolean; - /** - * Whether the collection allows the user to select all items via keyboard shortcut. - * @default false - */ - disallowSelectAll?: boolean; - /** - * Whether selection should occur automatically on focus. - * @default false - */ - selectOnFocus?: boolean; - /** - * Whether typeahead is disabled. - * @default false - */ - disallowTypeAhead?: boolean; - /** - * Whether the collection items should use virtual focus instead of being focused directly. - */ - shouldUseVirtualFocus?: boolean; - /** - * Whether navigation through tab key is enabled. - */ - allowsTabNavigation?: boolean; -} - -interface SelectableCollectionAria { - /** Props for the collection element. */ - collectionProps: HTMLAttributes; -} +import { useEffect } from "react"; +import { useSelectableCollection as useAriaSelectableCollection } from "@react-aria/selection"; /** - * Handles interactions with selectable collections. + * Wrapper around react-aria useSelectableCollection to fix pending issues or + * add missing functionality. */ -export function useSelectableCollection( - options: SelectableCollectionOptions -): SelectableCollectionAria { - let { - selectionManager: manager, - keyboardDelegate: delegate, - ref, - autoFocus = false, - shouldFocusWrap = false, - disallowEmptySelection = false, - disallowSelectAll = false, - selectOnFocus = false, - disallowTypeAhead = false, - shouldUseVirtualFocus, - allowsTabNavigation = false, - } = options; - let { direction } = useLocale(); - - let onKeyDown = (e: KeyboardEvent) => { - // Let child element (e.g. menu button) handle the event if the Alt key is pressed. - // Keyboard events bubble through portals. Don't handle keyboard events - // for elements outside the collection (e.g. menus). - if (e.altKey || !ref.current?.contains(e.target as HTMLElement)) { - return; - } - - const navigateToKey = ( - key: Key | undefined, - childFocus?: FocusStrategy - ) => { - if (key != null) { - manager.setFocusedKey(key, childFocus); - - if (e.shiftKey && manager.selectionMode === "multiple") { - manager.extendSelection(key); - } else if (selectOnFocus) { - manager.replaceSelection(key); - } - } - }; - - switch (e.key) { - case "ArrowDown": { - if (delegate.getKeyBelow) { - e.preventDefault(); - let nextKey = - manager.focusedKey != null - ? delegate.getKeyBelow(manager.focusedKey) - : delegate.getFirstKey?.(); - if (nextKey == null && shouldFocusWrap) { - nextKey = delegate.getFirstKey?.(manager.focusedKey); - } - // @ts-expect-error ignored strictness error in copied code - navigateToKey(nextKey); - } - break; - } - case "ArrowUp": { - if (delegate.getKeyAbove) { - e.preventDefault(); - let nextKey = - manager.focusedKey != null - ? delegate.getKeyAbove(manager.focusedKey) - : delegate.getLastKey?.(); - if (nextKey == null && shouldFocusWrap) { - nextKey = delegate.getLastKey?.(manager.focusedKey); - } - // @ts-expect-error ignored strictness error in copied code - navigateToKey(nextKey); - } - break; - } - case "ArrowLeft": { - if (delegate.getKeyLeftOf) { - e.preventDefault(); - let nextKey = delegate.getKeyLeftOf(manager.focusedKey); - // @ts-expect-error ignored strictness error in copied code - navigateToKey(nextKey, direction === "rtl" ? "first" : "last"); - } - break; - } - case "ArrowRight": { - if (delegate.getKeyRightOf) { - e.preventDefault(); - let nextKey = delegate.getKeyRightOf(manager.focusedKey); - // @ts-expect-error ignored strictness error in copied code - navigateToKey(nextKey, direction === "rtl" ? "last" : "first"); - } - break; - } - case "Home": - if (delegate.getFirstKey) { - e.preventDefault(); - let firstKey = delegate.getFirstKey( - manager.focusedKey, - isCtrlKeyPressed(e) - ); - // @ts-expect-error ignored strictness error in copied code - manager.setFocusedKey(firstKey); - if ( - isCtrlKeyPressed(e) && - e.shiftKey && - manager.selectionMode === "multiple" - ) { - // @ts-expect-error ignored strictness error in copied code - manager.extendSelection(firstKey); - } else if (selectOnFocus) { - // @ts-expect-error ignored strictness error in copied code - manager.replaceSelection(firstKey); - } - } - break; - case "End": - if (delegate.getLastKey) { - e.preventDefault(); - let lastKey = delegate.getLastKey( - manager.focusedKey, - isCtrlKeyPressed(e) - ); - // @ts-expect-error ignored strictness error in copied code - manager.setFocusedKey(lastKey); - if ( - isCtrlKeyPressed(e) && - e.shiftKey && - manager.selectionMode === "multiple" - ) { - // @ts-expect-error ignored strictness error in copied code - manager.extendSelection(lastKey); - } else if (selectOnFocus) { - // @ts-expect-error ignored strictness error in copied code - manager.replaceSelection(lastKey); - } - } - break; - case "PageDown": - if (delegate.getKeyPageBelow) { - e.preventDefault(); - let nextKey = delegate.getKeyPageBelow(manager.focusedKey); - // @ts-expect-error ignored strictness error in copied code - navigateToKey(nextKey); - } - break; - case "PageUp": - if (delegate.getKeyPageAbove) { - e.preventDefault(); - let nextKey = delegate.getKeyPageAbove(manager.focusedKey); - // @ts-expect-error ignored strictness error in copied code - navigateToKey(nextKey); - } - break; - case "a": - if ( - isCtrlKeyPressed(e) && - manager.selectionMode === "multiple" && - disallowSelectAll !== true - ) { - e.preventDefault(); - manager.selectAll(); - } - break; - case "Escape": - e.preventDefault(); - if (!disallowEmptySelection) { - manager.clearSelection(); - } - break; - case "Tab": { - if (!allowsTabNavigation) { - // There may be elements that are "tabbable" inside a collection (e.g. in a grid cell). - // However, collections should be treated as a single tab stop, with arrow key navigation internally. - // We don't control the rendering of these, so we can't override the tabIndex to prevent tabbing. - // Instead, we handle the Tab key, and move focus manually to the first/last tabbable element - // in the collection, so that the browser default behavior will apply starting from that element - // rather than the currently focused one. - if (e.shiftKey) { - ref.current.focus(); - } else { - let walker = getFocusableTreeWalker(ref.current, { - tabbable: true, - }); - let next: HTMLElement | undefined; - let last: HTMLElement; - do { - last = walker.lastChild() as HTMLElement; - if (last) { - next = last; - } - } while (last); - - if (next && !next.contains(document.activeElement)) { - next.focus(); - } - } - break; - } - } - } - }; - - let onFocus = (e: FocusEvent) => { - if (manager.isFocused) { - // If a focus event bubbled through a portal, reset focus state. - if (!e.currentTarget.contains(e.target)) { - manager.setFocused(false); - } - - return; - } - - // Focus events can bubble through portals. Ignore these events. - if (!e.currentTarget.contains(e.target)) { - return; - } - - manager.setFocused(true); - - if (manager.focusedKey == null) { - // If the user hasn't yet interacted with the collection, there will be no focusedKey set. - // Attempt to detect whether the user is tabbing forward or backward into the collection - // and either focus the first or last item accordingly. - let relatedTarget = e.relatedTarget as Element; - let key: Key | undefined; - if ( - relatedTarget && - e.currentTarget.compareDocumentPosition(relatedTarget) & - Node.DOCUMENT_POSITION_FOLLOWING - ) { - // @ts-expect-error ignored strictness error in copied code - key = manager.lastSelectedKey ?? delegate.getLastKey?.(); - } else { - // @ts-expect-error ignored strictness error in copied code - key = manager.firstSelectedKey ?? delegate.getFirstKey?.(); - } - if (key != null) { - manager.setFocusedKey(key); - } - } - }; - - let onBlur = (e: FocusEvent) => { - // Don't set blurred and then focused again if moving focus within the collection. - if (!e.currentTarget.contains(e.relatedTarget as HTMLElement)) { - manager.setFocused(false); - } - }; - +export const useSelectableCollection: typeof useAriaSelectableCollection = ( + options +) => { + const { + collectionProps: { onKeyDown, ...collectionProps }, + } = useAriaSelectableCollection(options); useEffect(() => { - if (autoFocus) { - let focusedKey = null; - - // Check focus strategy to determine which item to focus - if (autoFocus === "first") { - focusedKey = delegate.getFirstKey?.(); - } - if (autoFocus === "last") { - focusedKey = delegate.getLastKey?.(); - } - - // If there are any selected keys, make the first one the new focus target - let selectedKeys = manager.selectedKeys; - if (selectedKeys.size) { - focusedKey = selectedKeys.values().next().value; - } - - manager.setFocused(true); - manager.setFocusedKey(focusedKey); - - // If no default focus key is selected, focus the collection itself. - if (focusedKey == null && !shouldUseVirtualFocus && ref.current) { - focusSafely(ref.current); - } - - ///////////////////////////////////////////////////// MODIFICATION /////////////////////////////////////////////// - // Fixing https://github.com/adobe/react-spectrum/issues/4391 - if ( - options.selectOnFocus && - (autoFocus === "first" || autoFocus === "last") - ) { - manager.replaceSelection(focusedKey); - } - /////////////////////////////////////////////////// END OF MODIFICATION ////////////////////////////////////////// + // Fixing https://github.com/adobe/react-spectrum/issues/4391 + const { selectionManager, autoFocus, selectOnFocus } = options; + if ( + selectOnFocus && + (autoFocus === "first" || autoFocus === "last") && + selectionManager.focusedKey != null + ) { + selectionManager.replaceSelection(selectionManager.focusedKey); } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); - - let handlers = { - onKeyDown, - onFocus, - onBlur, - onMouseDown(e: React.MouseEvent) { - // Prevent focus going to the collection when clicking on the scrollbar. - e.preventDefault(); - }, - }; - - let { typeSelectProps } = useTypeSelect({ - keyboardDelegate: delegate, - selectionManager: manager, - onTypeSelect: selectOnFocus - ? (key) => { - manager.replaceSelection(key); - } - : undefined, }); - - if (!disallowTypeAhead) { - handlers = mergeProps(typeSelectProps, handlers); - } - - // If nothing is focused within the collection, make the collection itself tabbable. - // This will be marshalled to either the first or last item depending on where focus came from. - // If using virtual focus, don't set a tabIndex at all so that VoiceOver on iOS 14 doesn't try - // to move real DOM focus to the element anyway. - let tabIndex: number | undefined; - if (!shouldUseVirtualFocus) { - tabIndex = manager.focusedKey == null ? 0 : -1; - } - return { collectionProps: { - ...handlers, - tabIndex, + ...collectionProps, + onKeyDown: (e) => { + // keydown events used to not be handled when alt key is pressed. + // it's changed in https://github.com/adobe/react-spectrum/commit/885b5e6b84253925b2ac9f7f2766417c63b654b5#diff-e356ae602508922357fd9cd1aa7896f74e24f87bcb2ee86a7bb4a486ea87a2f2R120 + // handling keydown events when alt is down interferes with tree action shortcuts + // FIXME: make tree actions use capture phase to handle shortcuts so it takes precedence over + // the default handlers regardless of what the shortcut is. Revert the patch here, after. + // At the time of writing this, `useCapture` is only an option on ActionsProvider, but it + // needs to be an option on each action, to allow for this use case. + if (!e.altKey) { + onKeyDown?.(e); + } + }, }, }; -} +}; From f50ccab10251d31498bde3e36d7dabf529705e2f Mon Sep 17 00:00:00 2001 From: Alireza Date: Sun, 8 Dec 2024 00:41:15 +0100 Subject: [PATCH 10/17] fix(Tree): fix a regression in keyboard navigation Since @react-aria/selection@3.16.0 getKeyLeftOf and getKeyRightOf is removed if orientation is vertical and layout is "stack". Using the single-argument overload of the base constructor doesn't set a default value for orientation (which seems a little random), which in turn makes the if statement that removes the methods not run. https://github.com/adobe/react-spectrum/blob/8228e4efd9be99973058a1f90fc7f7377e673f78/packages/%40react-aria/selection/src/ListKeyboardDelegate.ts#L67 --- .../SpeedSearchTree/SpeedSearchTree.cy.tsx | 35 +++++++++++++++++++ packages/jui/src/Tree/Tree.cy.tsx | 35 +++++++++++++++++++ .../jui/src/Tree/TreeKeyboardDelegate.tsx | 12 +++++-- 3 files changed, 80 insertions(+), 2 deletions(-) diff --git a/packages/jui/src/Tree/SpeedSearchTree/SpeedSearchTree.cy.tsx b/packages/jui/src/Tree/SpeedSearchTree/SpeedSearchTree.cy.tsx index da1e272a..d911a7e9 100644 --- a/packages/jui/src/Tree/SpeedSearchTree/SpeedSearchTree.cy.tsx +++ b/packages/jui/src/Tree/SpeedSearchTree/SpeedSearchTree.cy.tsx @@ -9,6 +9,41 @@ const { Dynamic, HighlightsWithSpace } = composeStories(stories); const OS_NORMALIZED_META = Cypress.platform === "darwin" ? "Meta" : "Control"; describe("SpeedSearchTree", () => { + it("expands/collapses and navigates nodes by arrow keys", () => { + cy.mount( + + + node 1.1 + node 1.2 + + + ); + + cy.findByRole("treeitem") // only one should be visible initially + .click() // focus + .realPress("ArrowRight"); + // node 1 should be expanded + cy.findByRole("treeitem", { name: "node 1.1" }); + cy.findByRole("treeitem", { name: "node 1.2" }); + // but selection doesn't go to the children right away + cy.findByRole("treeitem", { name: "node 1", selected: true }); + cy.realPress("ArrowDown"); + cy.findByRole("treeitem", { name: "node 1.1", selected: true }); + cy.realPress("ArrowDown"); + cy.findByRole("treeitem", { name: "node 1.2", selected: true }); + cy.realPress("ArrowLeft"); // selection should move to node 1 but it remains expanded + cy.findByRole("treeitem", { name: "node 1", selected: true }); + cy.realPress("ArrowLeft"); // the second ArrowLeft closes the node + cy.findAllByRole("treeitem").should("have.length", 1); + + cy.realPress("ArrowRight"); // expanding the node again and going down with ArrowRight this time + cy.findByRole("treeitem", { name: "node 1", selected: true }); + cy.realPress("ArrowRight"); + cy.findByRole("treeitem", { name: "node 1.1", selected: true }); + cy.realPress("ArrowRight"); + cy.findByRole("treeitem", { name: "node 1.2", selected: true }); + }); + it("supports Speed Search in dynamic items mode", () => { cy.mount(); diff --git a/packages/jui/src/Tree/Tree.cy.tsx b/packages/jui/src/Tree/Tree.cy.tsx index 238f6706..1e414ec8 100644 --- a/packages/jui/src/Tree/Tree.cy.tsx +++ b/packages/jui/src/Tree/Tree.cy.tsx @@ -8,6 +8,41 @@ import { Item } from "../Collections"; const { Static, ScrollAndContainerWidth } = composeStories(stories); describe("Tree", () => { + it("expands/collapses and navigates nodes by arrow keys", () => { + cy.mount( + + + node 1.1 + node 1.2 + + + ); + + cy.findByRole("treeitem") // only one should be visible initially + .click() // focus + .realPress("ArrowRight"); + // node 1 should be expanded + cy.findByRole("treeitem", { name: "node 1.1" }); + cy.findByRole("treeitem", { name: "node 1.2" }); + // but selection doesn't go to the children right away + cy.findByRole("treeitem", { name: "node 1", selected: true }); + cy.realPress("ArrowDown"); + cy.findByRole("treeitem", { name: "node 1.1", selected: true }); + cy.realPress("ArrowDown"); + cy.findByRole("treeitem", { name: "node 1.2", selected: true }); + cy.realPress("ArrowLeft"); // selection should move to node 1 but it remains expanded + cy.findByRole("treeitem", { name: "node 1", selected: true }); + cy.realPress("ArrowLeft"); // the second ArrowLeft closes the node + cy.findAllByRole("treeitem").should("have.length", 1); + + cy.realPress("ArrowRight"); // expanding the node again and going down with ArrowRight this time + cy.findByRole("treeitem", { name: "node 1", selected: true }); + cy.realPress("ArrowRight"); + cy.findByRole("treeitem", { name: "node 1.1", selected: true }); + cy.realPress("ArrowRight"); + cy.findByRole("treeitem", { name: "node 1.2", selected: true }); + }); + it("opens nested expandable single-child items", () => { cy.mount(); diff --git a/packages/jui/src/Tree/TreeKeyboardDelegate.tsx b/packages/jui/src/Tree/TreeKeyboardDelegate.tsx index fed961d9..c6baa8a3 100644 --- a/packages/jui/src/Tree/TreeKeyboardDelegate.tsx +++ b/packages/jui/src/Tree/TreeKeyboardDelegate.tsx @@ -5,11 +5,19 @@ import React, { Key, RefObject } from "react"; export class TreeKeyboardDelegate extends ListKeyboardDelegate { constructor( private collection: Collection>, - private disabledKeys: Set, + disabledKeys: Set, ref: RefObject, collator?: Intl.Collator ) { - super(collection, disabledKeys, ref, collator); + super({ + collection, + disabledKeys, + ref, + collator, + // Since @react-aria/selection@3.16.0 getKeyLeftOf and getKeyRightOf is + // removed if orientation is vertical and layout is "stack". + layout: "grid", + }); } getKeyLeftOf(key: React.Key): React.Key { From 3c7ffdf19a5bd86b572f255ca8c9a2c82418fdb8 Mon Sep 17 00:00:00 2001 From: Alireza Date: Sun, 8 Dec 2024 00:42:43 +0100 Subject: [PATCH 11/17] docs(Tree): fix a keyboard navigation issue in tree stories if `title` is set on `Item`, the default getItemCollection method assumes the item's children contain child nodes, and sets hasChildNodes to `true`. --- packages/jui/src/Tree/Tree.stories.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/jui/src/Tree/Tree.stories.tsx b/packages/jui/src/Tree/Tree.stories.tsx index 9cb6b2ac..db81630b 100644 --- a/packages/jui/src/Tree/Tree.stories.tsx +++ b/packages/jui/src/Tree/Tree.stories.tsx @@ -94,7 +94,11 @@ export const Dynamic: StoryFn = () => { onSelectionChange={setSelectedKeys} > {(item) => ( - + {item.name} )} From a987a7afb8d61e8682536f1e3a1a6a3e87647404 Mon Sep 17 00:00:00 2001 From: Alireza Date: Sun, 8 Dec 2024 13:11:28 +0100 Subject: [PATCH 12/17] refactor(collections): simplify the API for connecting an input to a collection component useCollectionSearchInput had this advantage of allowing for connecting an input to ANY collection without requiring support from the collection component. However the collection component needed to support selectionManagerRef, so there was still a need to make the collection component compatible with useCollectionSearchInput. Plus, the usage API isn't simple enough, as it requires: - creating a ref to hold selection manager. Passing that ref to selectionManagerRef, and importing the right type for the ref value. - importing and invoking useCollectionSearchInput, and applying the returned props on the connected input. That's a lot of not-so-intuitive work. Considering any supported collection component still needs to support `CollectionRefProps` and utilize `useCollectionRef` to support connected input, the implementation is changed to have the event listeners baked into the collections (via a new useCollectionFocusProxy hook). This way at least the usage API is as simple as passing a ref to the connected input to `focusProxyRef` prop of the collection component. --- .../SearchEverywherePopup.tsx | 13 +- .../BranchesView/BranchesTree.tsx | 31 ++--- .../VersionControl/actions/PathInputField.tsx | 11 +- packages/jui/src/Collections/index.ts | 2 +- .../Collections/useCollectionFocusProxy.ts | 114 ++++++++++++++++++ .../Collections/useCollectionSearchInput.ts | 92 -------------- packages/jui/src/Dropdown/ListBox.tsx | 8 +- .../jui/src/Dropdown/StatelessListBox.tsx | 28 ++++- packages/jui/src/List/List.tsx | 4 +- packages/jui/src/List/story-helpers.tsx | 16 +-- packages/jui/src/List/useList.ts | 15 ++- .../Tree/SpeedSearchTree/SpeedSearchTree.tsx | 2 - packages/jui/src/Tree/Tree.tsx | 8 +- packages/jui/src/Tree/useSelectableTree.tsx | 16 ++- 14 files changed, 189 insertions(+), 171 deletions(-) create mode 100644 packages/jui/src/Collections/useCollectionFocusProxy.ts delete mode 100644 packages/jui/src/Collections/useCollectionSearchInput.ts diff --git a/packages/example-app/src/SearchEverywhere/SearchEverywherePopup.tsx b/packages/example-app/src/SearchEverywhere/SearchEverywherePopup.tsx index ae29d3f3..e1c1ffa1 100644 --- a/packages/example-app/src/SearchEverywhere/SearchEverywherePopup.tsx +++ b/packages/example-app/src/SearchEverywhere/SearchEverywherePopup.tsx @@ -17,7 +17,6 @@ import { Tabs, Tooltip, TooltipTrigger, - useCollectionSearchInput, useGetActionShortcut, } from "@intellij-platform/core"; import { useTips } from "./useTips"; @@ -276,7 +275,7 @@ export function SearchEverywherePopup() { const close = () => setOpen(false); - const collectionRef = useRef(null); + const searchInputRef = useRef(null); const selectionManagerRef = useRef(null); const onAction = (key: React.Key) => { if (key === LOAD_MORE_ITEM_KEY) { @@ -317,12 +316,6 @@ export function SearchEverywherePopup() { } }; - const { collectionSearchInputProps } = useCollectionSearchInput({ - collectionRef, - onAction, - selectionManagerRef, - }); - const tips = useTips(); return ( { event.target.select(); @@ -421,7 +414,7 @@ export function SearchEverywherePopup() { {pattern && ( (null); const searchInputRef = useRef(null); - const selectionManagerRef = useRef(null); const [isInputFocused, setInputFocused] = useState(false); const setBranchFilter = useSetRecoilState(vcsLogFilterCurrentTab.branch); const onAction = (key: React.Key) => { @@ -65,11 +57,6 @@ export function BranchesTree({ tabKey }: { tabKey: string }) { } }; - const { collectionSearchInputProps } = useCollectionSearchInput({ - collectionRef: ref, - selectionManagerRef, - onAction, - }); /** * TODO: remaining from search: * - Make the search input red when there is no match @@ -102,20 +89,18 @@ export function BranchesTree({ tabKey }: { tabKey: string }) { tabIndex={-1} value={searchTerm} onChange={(e) => setSearchTerm(e.target.value)} - {...mergeProps(collectionSearchInputProps, { - onFocus: () => { - setInputFocused(true); - }, - onBlur: () => { - setInputFocused(false); - }, - })} + onFocus={() => { + setInputFocused(true); + }} + onBlur={() => { + setInputFocused(false); + }} /> (null); const collectionRef = useRef(null); - const selectionManagerRef = useRef(null); const onAutocompleteSuggestionSelected = (key: React.Key) => { setValue(path.join(`${key}`, path.sep)); // TODO: handle completion with tab }; - const { collectionSearchInputProps } = useCollectionSearchInput({ - collectionRef, - onAction: onAutocompleteSuggestionSelected, - selectionManagerRef, - }); // Path search query is kept in a separate state because it needs to be updated when autocomplete action triggers. // Otherwise, it could be a computed value based on `isAutocompleteVisible` and `directory` const [pathSearchQuery, setPathSearchQuery] = useState(""); @@ -175,7 +167,6 @@ export function PathInputField({ inputProps={mergeProps( props.inputProps ?? {}, shortcutHandlerProps, - collectionSearchInputProps, { onKeyDown: (e: React.KeyboardEvent) => { if (e.key === "Escape") { @@ -218,7 +209,7 @@ export function PathInputField({ ; +} + +/** + * A solution for connecting a collection to a search input so that the collection can still be navigated by keyboard + * while the input is focused. It works by replaying certain keyboard events on the collection container and focused + * item. An alternative approach (which is used in react-aria's useCombobox) is to use useSelectableCollection + * separately for the input, but the biggest issue with that approach is that it's limiting in the following ways: + * - Rendering input should be a part of the same component that renders the collection. Having specific components + * for use cases that require a search input is not flexible enough. For example, one may want to use SpeedSearchList + * or List connected to an input. Also, the input and the collection may need to be in different layouts in different + * use cases. Decoupling the rendering of the input and collection is a more flexible solution. + * - The same options used for collection should be passed to the input field for behavior consistency, and that is + * prone to error. + * Some of these options, like `keyboardDelegate` can even have a default value in hooks like + * `useSelectableList`. + * It means making sure the same value is passed to the useSelectableCollection for input + * would require not using the default value, since the same value can't be accessed. + * + * With this event-forwarding approach, it's an arrow up or down event would behave exactly like it was triggered on + * the collection itself, leaving no room for behavior discrepancies. But it has a few drawbacks: + * - Although small, there is still some coupling between this code and implementation of the collection component. + * More specifically, this implementation assumes the following things: + * - "Enter" keys (selection or action) are handled on items, but arrow keys are handled on the collection element. + * - "[data-key] attribute is set on items. That is used to find the element for the focused item (which, of course, + * is not actually focused while the input is). + * + * Note: there has been some addition to react-aria useSelectableCollection and useSelectableItem hooks + * based on CustomEvent and a similar event reply mechanism in useAutocomplete. + * It may be possible to replace this hook with built-in functionality in react-aria at some point. + * But at the moment, it seems like that implementation is too coupled with the autocompletion use case, while + * what is supported here is more generic and allows for the connected search input use case too. + */ + +export const useCollectionFocusProxy = ({ + state, + focusProxyRef, + collectionRef, + onAction, +}: { + focusProxyRef: RefObject | undefined; + collectionRef: RefObject; + state: { + /** A collection of items in the list. */ + collection: Collection>; + /** A selection manager to read and update multiple selection state. */ + selectionManager: SelectionManager; + }; + onAction: ((key: Key) => void) | undefined; +}) => { + // TODO: focus/blur events should probably be handled as well, to keep the + // isFocused state of the collection in sync. + useEffect( + () => { + const proxy = focusProxyRef?.current; + if (proxy) { + const onKeyDown = (event: KeyboardEvent) => { + if (event.key === "ArrowUp" || event.key === "ArrowDown") { + event.preventDefault(); + event.stopPropagation(); + + collectionRef.current?.dispatchEvent( + new KeyboardEvent(event.type, event) + ); + } else if ( + event.key === "Enter" && + state.selectionManager?.focusedKey != null + ) { + event.preventDefault(); // in forms, pressing Enter on input submits the form + (event.currentTarget as HTMLElement)?.addEventListener( + "keyup", + (event: KeyboardEvent) => { + console.log( + "Keyup", + event.key, + state.selectionManager.focusedKey, + "onAction", + onAction + ); + if ( + event.key === "Enter" && + state.selectionManager.focusedKey != null + ) { + onAction?.(state.selectionManager.focusedKey); + } + }, + { once: true, capture: true } + ); + } + }; + proxy.addEventListener("keydown", onKeyDown); + return () => { + proxy.removeEventListener("keydown", onKeyDown); + }; + } + } /* with no dependency here, event listeners are reattached on each render, but that's the case when unmemoized + event handlers are passed to elements too (e.g., when using any react-aria hook) */ + ); +}; diff --git a/packages/jui/src/Collections/useCollectionSearchInput.ts b/packages/jui/src/Collections/useCollectionSearchInput.ts deleted file mode 100644 index 1f0e4597..00000000 --- a/packages/jui/src/Collections/useCollectionSearchInput.ts +++ /dev/null @@ -1,92 +0,0 @@ -import React, { Key, RefObject } from "react"; -import { SelectionManager } from "@react-stately/selection"; -import { useEventCallback } from "@intellij-platform/core/utils/useEventCallback"; -import { DOMAttributes } from "@react-types/shared"; - -/** - * A solution for connecting a collection to a search input, so that collection can still be navigated by keyboard - * while the input is focused. It works by replaying certain keyboard events on the collection container and focused - * item. An alternative approach (which is used in react-aria's useCombobox) is to use useSelectableCollection - * separately for the input, but the biggest issue with that approach is that it's limiting in the following ways: - * - Rendering input should be a part of the same component that renders the collection. Having specific components - * for use cases that requires a search input is not flexible enough. For example one may want to use SpeedSearchList - * or List connected to an input. Also, the input and the collection may need to be in different layouts in different - * use cases. Decoupling the rendering of the input and collection is a more flexible solution. - * - The same options used for collection should be passed to the input field for behavior consistency, and that can be - * prone to error. Some of these options, like `keyboardDelegate` can even have a default value in hooks like - * `useSelectableList`, which means for making sure the same value is passed to the useSelectableCollection for input, - * would require to not use the default value, since the same value can't be accessed. - * - * With this event forwarding approach, it's an arrow up or down event would behave exactly like it was triggered on - * the collection itself, leaving no room for behavior discrepancies. But it has a few drawbacks: - * - Although small, there is still some coupling between this code and implementation of the collection component. - * More specifically, the following things are assumed by this implementation: - * - "Enter" keys (selection or action) are handled on items, but arrow keys are handled on the collection element. - * - "[data-key] attribute is set on items. That is used to find the element for the focused item (which of course is - * not actually focused while the input is). - */ -export const useCollectionSearchInput = ({ - collectionRef, - selectionManagerRef, - onAction, -}: { - /** - * ref to the html element of the collection component - */ - collectionRef: RefObject; - /** - * SelectionManager instance, returned from the state management hook for the collection component. - * {@link CollectionRefProps.selectionManagerRef} can be used on collection components that implement - * `useCollectionRef`, to get a hold of selection manager, from outside. - */ - selectionManagerRef: RefObject | null | undefined; - /** - * onAction callback passed to the collection component. It's needed since some upgrade of @react-aria/interactions, - * since a check is added to not have keyup events on outside elements trigger onPress. That's to prevent scenarios - * where focus is moved between keydown and keyup, but is also breaking the previous solution of just replying - * input keyboard events on the list item. - * @param key - */ - onAction?: (key: Key) => void; -}): { collectionSearchInputProps: DOMAttributes } => { - const relayEventsToCollection = useEventCallback( - (event: React.KeyboardEvent) => { - // Relay ArrowUp and ArrowDown to the container - if ( - event.type === "keydown" && - (event.key === "ArrowUp" || event.key === "ArrowDown") - ) { - event.preventDefault(); - event.stopPropagation(); - collectionRef.current?.dispatchEvent( - new KeyboardEvent(event.type, event.nativeEvent) - ); - } else if ( - event.type === "keydown" && - event.key === "Enter" && - selectionManagerRef?.current?.focusedKey != null - ) { - event.preventDefault(); // in forms, pressing Enter on input submits the form - event.currentTarget.addEventListener( - "keyup", - (event: KeyboardEvent) => { - if ( - event.key === "Enter" && - selectionManagerRef?.current?.focusedKey != null - ) { - onAction?.(selectionManagerRef?.current?.focusedKey); - } - }, - { once: true, capture: true } - ); - } - } - ); - - return { - collectionSearchInputProps: { - onKeyDown: relayEventsToCollection, - onKeyPress: relayEventsToCollection, - }, - }; -}; diff --git a/packages/jui/src/Dropdown/ListBox.tsx b/packages/jui/src/Dropdown/ListBox.tsx index d4a1caeb..62f5d1c3 100644 --- a/packages/jui/src/Dropdown/ListBox.tsx +++ b/packages/jui/src/Dropdown/ListBox.tsx @@ -2,14 +2,11 @@ import React, { ForwardedRef } from "react"; import { AriaListBoxOptions, AriaListBoxProps } from "@react-aria/listbox"; import { useListState } from "@react-stately/list"; import { StatelessListBox } from "@intellij-platform/core/Dropdown/StatelessListBox"; -import { - CollectionRefProps, - useCollectionRef, -} from "@intellij-platform/core/Collections/useCollectionRef"; +import { CollectionFocusProxyProps } from "@intellij-platform/core/Collections"; export interface ListBoxProps extends AriaListBoxProps, - CollectionRefProps, + CollectionFocusProxyProps, Pick< AriaListBoxOptions, "shouldFocusOnHover" | "shouldUseVirtualFocus" @@ -26,7 +23,6 @@ export const ListBox = React.forwardRef(function ListBox( selectionMode: "single", ...props, }); - useCollectionRef(props, state); return ; }); diff --git a/packages/jui/src/Dropdown/StatelessListBox.tsx b/packages/jui/src/Dropdown/StatelessListBox.tsx index 3b260b08..fd11ba83 100644 --- a/packages/jui/src/Dropdown/StatelessListBox.tsx +++ b/packages/jui/src/Dropdown/StatelessListBox.tsx @@ -9,12 +9,24 @@ import { useObjectRef } from "@react-aria/utils"; import { ListState } from "@react-stately/list"; import { Node } from "@react-types/shared"; -import { ItemStateContext } from "@intellij-platform/core/Collections"; +import { + CollectionFocusProxyProps, + ItemStateContext, + useCollectionFocusProxy, +} from "@intellij-platform/core/Collections"; import { StyledListItem } from "@intellij-platform/core/List/StyledListItem"; import { StyledList } from "@intellij-platform/core/List/StyledList"; import { StyledVerticalSeparator } from "@intellij-platform/core/StyledSeparator"; import { styled } from "@intellij-platform/core/styled"; +interface StatelessListBoxProps + extends AriaListBoxOptions, + CollectionFocusProxyProps { + state: ListState; + style?: CSSProperties; + className?: string; +} + export const StatelessListBox = React.forwardRef(function StatelessListBox< T extends object >( @@ -22,17 +34,21 @@ export const StatelessListBox = React.forwardRef(function StatelessListBox< state, style, className, + focusProxyRef, ...props - }: AriaListBoxOptions & { - state: ListState; - style?: CSSProperties; - className?: string; - }, + }: StatelessListBoxProps, forwardedRef: ForwardedRef ) { const ref = useObjectRef(forwardedRef); const { listBoxProps, labelProps } = useListBox(props, state, ref); + useCollectionFocusProxy({ + state, + collectionRef: ref, + focusProxyRef, + onAction: props.onAction, + }); + return ( <>
{props.label}
diff --git a/packages/jui/src/List/List.tsx b/packages/jui/src/List/List.tsx index 411de626..0b97bb1c 100644 --- a/packages/jui/src/List/List.tsx +++ b/packages/jui/src/List/List.tsx @@ -11,12 +11,14 @@ import { CollectionRefProps } from "@intellij-platform/core/Collections/useColle import { Virtualizer } from "@react-aria/virtualizer"; import { useListVirtualizer } from "@intellij-platform/core/List/useListVirtualizer"; import { ListContext } from "@intellij-platform/core/List/ListContext"; +import { CollectionFocusProxyProps } from "@intellij-platform/core/Collections"; export type ListProps = Omit< Omit, "disallowEmptySelection">, keyof AsyncLoadable > & - CollectionRefProps & { + CollectionRefProps & + CollectionFocusProxyProps & { /** * fills the available horizontal or vertical space, when rendered in a flex container. */ diff --git a/packages/jui/src/List/story-helpers.tsx b/packages/jui/src/List/story-helpers.tsx index f7d5217e..ae7cd087 100644 --- a/packages/jui/src/List/story-helpers.tsx +++ b/packages/jui/src/List/story-helpers.tsx @@ -1,7 +1,6 @@ import { Legend, legends } from "../../test-data"; -import React, { ReactNode } from "react"; +import React, { ReactNode, useRef } from "react"; import { Story } from "@storybook/react"; -import { SelectionManager } from "@react-stately/selection"; import { Divider, DividerItem, @@ -10,7 +9,6 @@ import { List, ListProps, Section, - useCollectionSearchInput, } from "@intellij-platform/core"; import { Pane } from "../story-components"; @@ -55,24 +53,20 @@ export const renderItemTextWithHighlights = (item: Legend) => ( export const commonListStories = { withConnectedInput: (ListCmp: typeof List) => { const WithConnectedInput: Story> = (props) => { + const inputRef = useRef(null); const [isFocused, setIsFocused] = React.useState(false); const listRef = React.useRef(null); - const selectionManagerRef = React.useRef(null); - const { collectionSearchInputProps } = useCollectionSearchInput({ - collectionRef: listRef, - onAction: props.onAction, - selectionManagerRef, - }); + return ( setIsFocused(true)} onBlur={() => setIsFocused(false)} /> , + CollectionFocusProxyProps, CollectionRefProps { allowEmptySelection?: boolean; /** @@ -31,7 +36,7 @@ export interface ListProps // import { useSelectableList } from "@react-aria/selection"; export function useList( - { onAction, showAsFocused, ...props }: ListProps, + { onAction, showAsFocused, focusProxyRef, ...props }: ListProps, state: ListState, ref: React.RefObject ) { @@ -47,6 +52,14 @@ export function useList( // if selectOnFocus is going to be an option (which is not in intellij UI), we should also conditionally show outline on items selectOnFocus: true, }); + + useCollectionFocusProxy({ + focusProxyRef, + onAction, + state, + collectionRef: ref, + }); + const [focused, setFocused] = useState(false); const { focusWithinProps } = useFocusWithin({ diff --git a/packages/jui/src/Tree/SpeedSearchTree/SpeedSearchTree.tsx b/packages/jui/src/Tree/SpeedSearchTree/SpeedSearchTree.tsx index 25fd0ae4..65082ce1 100644 --- a/packages/jui/src/Tree/SpeedSearchTree/SpeedSearchTree.tsx +++ b/packages/jui/src/Tree/SpeedSearchTree/SpeedSearchTree.tsx @@ -6,7 +6,6 @@ import { SpeedSearchProps, SpeedSearchStateProps, } from "@intellij-platform/core/SpeedSearch"; -import { useCollectionRef } from "@intellij-platform/core/Collections/useCollectionRef"; import useForwardedRef from "@intellij-platform/core/utils/useForwardedRef"; import { StyledTree } from "../StyledTree"; import { SpeedSearchPopup } from "../../SpeedSearch/SpeedSearchPopup"; @@ -44,7 +43,6 @@ export const SpeedSearchTree = React.forwardRef( { ...props, disallowEmptySelection: !props.allowEmptySelection }, treeRef ); - useCollectionRef(props, state); const ref = useForwardedRef(forwardedRef); const { treeProps, diff --git a/packages/jui/src/Tree/Tree.tsx b/packages/jui/src/Tree/Tree.tsx index d44ef2a2..e28fa679 100644 --- a/packages/jui/src/Tree/Tree.tsx +++ b/packages/jui/src/Tree/Tree.tsx @@ -5,20 +5,15 @@ import { StyledTree } from "./StyledTree"; import { TreeRefValue } from "./useTreeRef"; import { TreeNode } from "./TreeNode"; import { TreeContext } from "./TreeContext"; -import { useTreeState, TreeProps as StatelyTreeProps } from "./useTreeState"; +import { TreeProps as StatelyTreeProps, useTreeState } from "./useTreeState"; import { SelectableTreeProps, useSelectableTree } from "./useSelectableTree"; import { useTreeVirtualizer } from "./useTreeVirtualizer"; import { CollectionCacheInvalidationProps } from "@intellij-platform/core/Collections/useCollectionCacheInvalidation"; -import { - CollectionRefProps, - useCollectionRef, -} from "@intellij-platform/core/Collections/useCollectionRef"; import { filterDOMProps, useObjectRef } from "@react-aria/utils"; export interface TreeProps extends Omit, "disallowEmptySelection">, CollectionCacheInvalidationProps, - CollectionRefProps, Omit, "keyboardDelegate" | "isVirtualized"> { fillAvailableSpace?: boolean; /** @@ -52,7 +47,6 @@ export const Tree = React.forwardRef( forwardedRef: ForwardedRef ) => { const state = useTreeState(props, treeRef); - useCollectionRef(props, state); const ref = useObjectRef(forwardedRef); const { treeProps, treeContext } = useSelectableTree( diff --git a/packages/jui/src/Tree/useSelectableTree.tsx b/packages/jui/src/Tree/useSelectableTree.tsx index 7b08c292..3272ac9e 100644 --- a/packages/jui/src/Tree/useSelectableTree.tsx +++ b/packages/jui/src/Tree/useSelectableTree.tsx @@ -18,10 +18,15 @@ import { hasAnyModifier } from "@intellij-platform/core/utils/keyboard-utils"; import { FocusEvents } from "@react-types/shared/src/events"; import { FocusStrategy } from "@react-types/shared/src/selection"; import { groupBy } from "ramda"; +import { + CollectionFocusProxyProps, + useCollectionFocusProxy, +} from "@intellij-platform/core/Collections"; export interface SelectableTreeProps extends DOMProps, - Omit { + Omit, + CollectionFocusProxyProps { isVirtualized?: boolean; keyboardDelegate?: KeyboardDelegate; /** @@ -49,6 +54,7 @@ export function useSelectableTree( onBlur, autoFocus, showAsFocused, + focusProxyRef, ...props }: SelectableTreeProps, state: TreeState, @@ -84,6 +90,14 @@ export function useSelectableTree( [state.collection, state.disabledKeys, props.keyboardDelegate] ), }); + + useCollectionFocusProxy({ + collectionRef: ref, + state, + onAction, + focusProxyRef, + }); + const { focusWithinProps } = useFocusWithin({ onFocusWithinChange: setFocused, }); From 067018447cd0335afe4747c9ea80a32cda3272ce Mon Sep 17 00:00:00 2001 From: Alireza Date: Mon, 9 Dec 2024 19:59:05 +0100 Subject: [PATCH 13/17] fix a build error --- packages/jui/src/Collections/index.ts | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/jui/src/Collections/index.ts b/packages/jui/src/Collections/index.ts index cb338426..fd8249ad 100644 --- a/packages/jui/src/Collections/index.ts +++ b/packages/jui/src/Collections/index.ts @@ -1,11 +1,19 @@ +export { + type CollectionFocusProxyProps, + useCollectionFocusProxy, +} from "./useCollectionFocusProxy"; +export { Divider, DividerItem } from "./Divider"; +export * from "./ItemStateContext"; +export * from "./ItemLayout"; +export { Item } from "./Item"; + +// Exporting from third-party modules leads to weird "@parcel/transformer-typescript-types: Got unexpected undefined" +// error. +// It's probably a bug in Parcel maybe triggered by something like a circular dependency. +// FIXME: remove this comment and reorder exports when the build tool is changed (to Rolldown or Vite or...) export { Section } from "@react-stately/collections"; export { SelectionManager } from "@react-stately/selection"; export { type Selection } from "@react-types/shared"; -export { Item } from "./Item"; -export * from "./Divider"; -export * from "./ItemStateContext"; -export * from "./ItemLayout"; -export * from "./useCollectionFocusProxy"; // NOTE: some stuff like `useCollectionCacheInvalidation` are not exported from the index, since the index is re-exported // from other modules like Menu, Tabs, etc., but that part of the API is not considered a public API at the moment. From 05b8f53f126c5868b2d3ada6d28ba273a8c66b68 Mon Sep 17 00:00:00 2001 From: Alireza Date: Mon, 9 Dec 2024 22:15:58 +0100 Subject: [PATCH 14/17] upgrade @react-aria/selection to prevent a random error in running tests (uncaught exception)TypeError: Cannot read properties of null (reading 'scrollTop') caused only when running tests on CI pipeline. Due to ref being null, which is safe-guarded with optional chaining in @react-aria/selection@3.21.0 https://cloud.cypress.io/projects/o1ooqz/runs/253/test-results/f5726287-8ab4-4441-836b-763b32845987/replay?actions=%5B%5D&browsers=%5B%5D&groups=%5B%5D&isFlaky=%5B%5D&modificationDateRange=%7B%22startDate%22%3A%221970-01-01%22%2C%22endDate%22%3A%222038-01-19%22%7D&orderBy=EXECUTION_ORDER&oses=%5B%5D&specs=%5B%5D&statuses=%5B%7B%22value%22%3A%22FAILED%22%2C%22label%22%3A%22FAILED%22%7D%5D&testingTypesEnum=%5B%5D&ts=1733774625108.0454&utm_source=github&att=1 --- packages/jui/package.json | 12 +- .../jui/src/Tree/TreeKeyboardDelegate.tsx | 4 +- yarn.lock | 286 ++++++++---------- 3 files changed, 141 insertions(+), 161 deletions(-) diff --git a/packages/jui/package.json b/packages/jui/package.json index ec2376ee..966e7394 100644 --- a/packages/jui/package.json +++ b/packages/jui/package.json @@ -45,9 +45,9 @@ "@react-aria/checkbox": "~3.3.4", "@react-aria/combobox": "^3.10.3", "@react-aria/dialog": "~3.2.1", - "@react-aria/focus": "~3.18.1", - "@react-aria/i18n": "^3.12.1", - "@react-aria/interactions": "~3.22.1", + "@react-aria/focus": "~3.19.0", + "@react-aria/i18n": "^3.12.4", + "@react-aria/interactions": "~3.22.5", "@react-aria/label": "~3.7.10", "@react-aria/link": "~3.2.5", "@react-aria/listbox": "~3.12.1", @@ -55,14 +55,14 @@ "@react-aria/overlays": "~3.23.1", "@react-aria/progress": "~3.1.8", "@react-aria/select": "~3.14.7", - "@react-aria/selection": "~3.18.1", + "@react-aria/selection": "~3.21.0", "@react-aria/separator": "~3.1.7", "@react-aria/tabs": "~3.1.5", "@react-aria/tooltip": "~3.3.4", "@react-aria/utils": "~3.25.1", "@react-aria/virtualizer": "3.3.7", "@react-aria/visually-hidden": "~3.8.14", - "@react-stately/collections": "~3.10.9", + "@react-stately/collections": "~3.12.0", "@react-stately/combobox": "^3.9.2", "@react-stately/layout": "3.4.4", "@react-stately/list": "~3.10.7", @@ -77,7 +77,7 @@ "@react-stately/utils": "~3.10.2", "@react-stately/virtualizer": "~3.1.9", "@react-types/menu": "~3.9.11", - "@react-types/shared": "~3.24.1", + "@react-types/shared": "~3.26.0", "@swc/helpers": "~0.3.17", "ramda": "~0.27.2" }, diff --git a/packages/jui/src/Tree/TreeKeyboardDelegate.tsx b/packages/jui/src/Tree/TreeKeyboardDelegate.tsx index c6baa8a3..eb1f5846 100644 --- a/packages/jui/src/Tree/TreeKeyboardDelegate.tsx +++ b/packages/jui/src/Tree/TreeKeyboardDelegate.tsx @@ -20,12 +20,12 @@ export class TreeKeyboardDelegate extends ListKeyboardDelegate { }); } - getKeyLeftOf(key: React.Key): React.Key { + getKeyLeftOf(key: React.Key): React.Key | null { const item = this.collection.getItem(key); return item?.parentKey ?? this.getKeyAbove(key); } - getKeyRightOf(key: React.Key): React.Key { + getKeyRightOf(key: React.Key): React.Key | null { return this.getKeyBelow(key); } } diff --git a/yarn.lock b/yarn.lock index 3264a63f..4e55cf35 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2903,9 +2903,9 @@ __metadata: "@react-aria/checkbox": ~3.3.4 "@react-aria/combobox": ^3.10.3 "@react-aria/dialog": ~3.2.1 - "@react-aria/focus": ~3.18.1 - "@react-aria/i18n": ^3.12.1 - "@react-aria/interactions": ~3.22.1 + "@react-aria/focus": ~3.19.0 + "@react-aria/i18n": ^3.12.4 + "@react-aria/interactions": ~3.22.5 "@react-aria/label": ~3.7.10 "@react-aria/link": ~3.2.5 "@react-aria/listbox": ~3.12.1 @@ -2913,14 +2913,14 @@ __metadata: "@react-aria/overlays": ~3.23.1 "@react-aria/progress": ~3.1.8 "@react-aria/select": ~3.14.7 - "@react-aria/selection": ~3.18.1 + "@react-aria/selection": ~3.21.0 "@react-aria/separator": ~3.1.7 "@react-aria/tabs": ~3.1.5 "@react-aria/tooltip": ~3.3.4 "@react-aria/utils": ~3.25.1 "@react-aria/virtualizer": 3.3.7 "@react-aria/visually-hidden": ~3.8.14 - "@react-stately/collections": ~3.10.9 + "@react-stately/collections": ~3.12.0 "@react-stately/combobox": ^3.9.2 "@react-stately/data": ^3.11.6 "@react-stately/layout": 3.4.4 @@ -2939,7 +2939,7 @@ __metadata: "@react-types/link": ^3.5.7 "@react-types/listbox": 3.5.1 "@react-types/menu": ~3.9.11 - "@react-types/shared": ~3.24.1 + "@react-types/shared": ~3.26.0 "@storybook/addon-actions": ^7.3.2 "@storybook/addon-essentials": ^7.3.2 "@storybook/addon-links": ^7.3.2 @@ -2982,40 +2982,40 @@ __metadata: languageName: unknown linkType: soft -"@internationalized/date@npm:^3.5.5": - version: 3.5.5 - resolution: "@internationalized/date@npm:3.5.5" +"@internationalized/date@npm:^3.6.0": + version: 3.6.0 + resolution: "@internationalized/date@npm:3.6.0" dependencies: "@swc/helpers": ^0.5.0 - checksum: 610afabe7d03f55d12126798c1f853a4f244e8567c3bcc66b1da2ae1bce376aad12876dc5019a949f2a8fe3a492cd2b4d354b9350a45fec3f7c5c7ff81401fc6 + checksum: 82a66c7d7eef8bc49c4ee5e99ecfa91a4752a3a96296a34c5549fe3fb98c5d37c3688887a253ffb991749d3425f7045c7c6b24c4f98c4929d0ef7f8312fa68ec languageName: node linkType: hard -"@internationalized/message@npm:^3.1.4": - version: 3.1.4 - resolution: "@internationalized/message@npm:3.1.4" +"@internationalized/message@npm:^3.1.6": + version: 3.1.6 + resolution: "@internationalized/message@npm:3.1.6" dependencies: "@swc/helpers": ^0.5.0 intl-messageformat: ^10.1.0 - checksum: 37990cf4fd666afe8d165f3c9042e88c2d95b4a03d6e67595a49d57aff938d19f91826c7c6e5cfa2863c3d8d555365e797d5979da575a835b533fd2e31876bef + checksum: a291d32e797a3694d1279c4fb74f2812991f007b15fbd67e148d2089339a4f3e11b4803eae6f1cc4ae1a1872b39bdcafe30f9bb365accdf5ed2af063e532d00f languageName: node linkType: hard -"@internationalized/number@npm:^3.5.3": - version: 3.5.3 - resolution: "@internationalized/number@npm:3.5.3" +"@internationalized/number@npm:^3.6.0": + version: 3.6.0 + resolution: "@internationalized/number@npm:3.6.0" dependencies: "@swc/helpers": ^0.5.0 - checksum: f905cb5302d5a84660fbe0264930fadf286c7a5860373c289863bc2b9d003690552743da2b3155d65e3e9fd0e49b83673caf49c306b9bab39d6e871b6777c588 + checksum: 764078650ac562a54a22938d6889ed2cb54e411a4c58b098dabc8514572709bbc206f8e44b50bd684600e454b0276c2617ddc6d9a7345521f2896a13b1c085a7 languageName: node linkType: hard -"@internationalized/string@npm:^3.2.3": - version: 3.2.3 - resolution: "@internationalized/string@npm:3.2.3" +"@internationalized/string@npm:^3.2.5": + version: 3.2.5 + resolution: "@internationalized/string@npm:3.2.5" dependencies: "@swc/helpers": ^0.5.0 - checksum: aad1dd1de52fa48f17e41ad0a502bab621a08aadb8ccfc02512211d05f7111920d094b49811394a930542a98fe22522c2b5818f6d64eb38aca9638b7b4f11ccd + checksum: e1ad90f418e8a35f49b6fe91cc91ea5230083808b337feaff60f8a0a8a32ee33895728bc4024cdfe93bf6596b3a3dc72cd5f8b7daba29962fbc68827c816fecd languageName: node linkType: hard @@ -5767,33 +5767,18 @@ __metadata: languageName: node linkType: hard -"@react-aria/focus@npm:^3.10.1, @react-aria/focus@npm:^3.17.1, @react-aria/focus@npm:^3.18.1, @react-aria/focus@npm:^3.5.5, @react-aria/focus@npm:^3.6.1, @react-aria/focus@npm:~3.18.1": - version: 3.18.1 - resolution: "@react-aria/focus@npm:3.18.1" +"@react-aria/focus@npm:^3.10.1, @react-aria/focus@npm:^3.17.1, @react-aria/focus@npm:^3.18.1, @react-aria/focus@npm:^3.18.2, @react-aria/focus@npm:^3.19.0, @react-aria/focus@npm:^3.5.5, @react-aria/focus@npm:^3.6.1, @react-aria/focus@npm:~3.19.0": + version: 3.19.0 + resolution: "@react-aria/focus@npm:3.19.0" dependencies: - "@react-aria/interactions": ^3.22.1 - "@react-aria/utils": ^3.25.1 - "@react-types/shared": ^3.24.1 + "@react-aria/interactions": ^3.22.5 + "@react-aria/utils": ^3.26.0 + "@react-types/shared": ^3.26.0 "@swc/helpers": ^0.5.0 clsx: ^2.0.0 peerDependencies: - react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0 - checksum: 2534bd3b3572dcb7716988817922542948bddc7ddb717aaeda9a93b90b39f8d367521ff59e985f02aa4da96cb3ce764e786817dedea0ca6187e963a949e72090 - languageName: node - linkType: hard - -"@react-aria/focus@npm:^3.18.2": - version: 3.18.2 - resolution: "@react-aria/focus@npm:3.18.2" - dependencies: - "@react-aria/interactions": ^3.22.2 - "@react-aria/utils": ^3.25.2 - "@react-types/shared": ^3.24.1 - "@swc/helpers": ^0.5.0 - clsx: ^2.0.0 - peerDependencies: - react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0 - checksum: 1d94207f2b956fb181f1bbc8e74c244fd2398b75c38ae2c7691ab7c67b1ab831563ce2d1ec01db95b0cfeffbf080889dbea251b6a9f12d8fc3854925b874b32c + react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 + checksum: 5109b24a89ba049cf3b9ffc71ad68fedd8d667a8d9a50a41f334d97db01abf22d144b32ff1ca68f76b7067d9a67e27d5cb13989cd92fcd3734e4e509a04c9ad5 languageName: node linkType: hard @@ -5827,67 +5812,35 @@ __metadata: languageName: node linkType: hard -"@react-aria/i18n@npm:^3.11.1, @react-aria/i18n@npm:^3.12.1, @react-aria/i18n@npm:^3.3.6, @react-aria/i18n@npm:^3.3.9": - version: 3.12.1 - resolution: "@react-aria/i18n@npm:3.12.1" - dependencies: - "@internationalized/date": ^3.5.5 - "@internationalized/message": ^3.1.4 - "@internationalized/number": ^3.5.3 - "@internationalized/string": ^3.2.3 - "@react-aria/ssr": ^3.9.5 - "@react-aria/utils": ^3.25.1 - "@react-types/shared": ^3.24.1 - "@swc/helpers": ^0.5.0 - peerDependencies: - react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0 - checksum: 85c0c111301ec2f6caeb7e2c2b8988dcf4e46a4be76684ede0834252f9a5fb862d8ab63284773828d6652ce3af7b30fcb22d4a9e958bdc8a812d75c458f2903f - languageName: node - linkType: hard - -"@react-aria/i18n@npm:^3.12.2": - version: 3.12.2 - resolution: "@react-aria/i18n@npm:3.12.2" +"@react-aria/i18n@npm:^3.11.1, @react-aria/i18n@npm:^3.12.1, @react-aria/i18n@npm:^3.12.2, @react-aria/i18n@npm:^3.12.4, @react-aria/i18n@npm:^3.3.6, @react-aria/i18n@npm:^3.3.9": + version: 3.12.4 + resolution: "@react-aria/i18n@npm:3.12.4" dependencies: - "@internationalized/date": ^3.5.5 - "@internationalized/message": ^3.1.4 - "@internationalized/number": ^3.5.3 - "@internationalized/string": ^3.2.3 - "@react-aria/ssr": ^3.9.5 - "@react-aria/utils": ^3.25.2 - "@react-types/shared": ^3.24.1 + "@internationalized/date": ^3.6.0 + "@internationalized/message": ^3.1.6 + "@internationalized/number": ^3.6.0 + "@internationalized/string": ^3.2.5 + "@react-aria/ssr": ^3.9.7 + "@react-aria/utils": ^3.26.0 + "@react-types/shared": ^3.26.0 "@swc/helpers": ^0.5.0 peerDependencies: - react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0 - checksum: 53e36b1e93c80b8a227f750d6fd46e8bdfe41added599d3fa70602707d37895ca5a2cfc8cc922cf42d8835220a1ae8ee11234b3da6b17e3c4668c21b476d31b2 + react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 + checksum: a3d9593bdef208d8b2c23b78e89b9d7a62f62755ef66a77e7b33919fabcbf232f8e4f90ab1d373776487aef461d3b7650b89c33480f1915a1f23184182b062ae languageName: node linkType: hard -"@react-aria/interactions@npm:^3.13.1, @react-aria/interactions@npm:^3.21.3, @react-aria/interactions@npm:^3.22.1, @react-aria/interactions@npm:^3.8.4, @react-aria/interactions@npm:~3.22.1": - version: 3.22.1 - resolution: "@react-aria/interactions@npm:3.22.1" +"@react-aria/interactions@npm:^3.13.1, @react-aria/interactions@npm:^3.21.3, @react-aria/interactions@npm:^3.22.1, @react-aria/interactions@npm:^3.22.2, @react-aria/interactions@npm:^3.22.5, @react-aria/interactions@npm:^3.8.4, @react-aria/interactions@npm:~3.22.5": + version: 3.22.5 + resolution: "@react-aria/interactions@npm:3.22.5" dependencies: - "@react-aria/ssr": ^3.9.5 - "@react-aria/utils": ^3.25.1 - "@react-types/shared": ^3.24.1 + "@react-aria/ssr": ^3.9.7 + "@react-aria/utils": ^3.26.0 + "@react-types/shared": ^3.26.0 "@swc/helpers": ^0.5.0 peerDependencies: - react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0 - checksum: 57790dbc823b23f137aedbcb8d6d9d0392689312a932baa8fa4a73c23e6865920b3c451543659afde917b34caef18601ca23079aeba0a8e00dff02b0f3e6db80 - languageName: node - linkType: hard - -"@react-aria/interactions@npm:^3.22.2": - version: 3.22.2 - resolution: "@react-aria/interactions@npm:3.22.2" - dependencies: - "@react-aria/ssr": ^3.9.5 - "@react-aria/utils": ^3.25.2 - "@react-types/shared": ^3.24.1 - "@swc/helpers": ^0.5.0 - peerDependencies: - react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0 - checksum: c856ebcec096bfeaa347def1e702a341f4bc0edbbc4bb4b8a72393bf728b02da03369afc2dd5d878a426afab84ac5b63c1dd326d59f0ce1d50d3ef94742a5966 + react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 + checksum: f15c8c343ad6f9725a801a2d931a59a9a0302cd4577e542c3d57ebc1296fcdbd2c75f7cd9f36516ff838f3f3afa2ef2414ba0a514d97663043b7ec07ac8a1611 languageName: node linkType: hard @@ -6159,57 +6112,21 @@ __metadata: languageName: node linkType: hard -"@react-aria/selection@npm:^3.18.1, @react-aria/selection@npm:^3.19.1, @react-aria/selection@npm:^3.8.2": - version: 3.19.1 - resolution: "@react-aria/selection@npm:3.19.1" - dependencies: - "@react-aria/focus": ^3.18.1 - "@react-aria/i18n": ^3.12.1 - "@react-aria/interactions": ^3.22.1 - "@react-aria/utils": ^3.25.1 - "@react-stately/selection": ^3.16.1 - "@react-types/shared": ^3.24.1 - "@swc/helpers": ^0.5.0 - peerDependencies: - react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0 - react-dom: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0 - checksum: 5fad769ed612d378cf4bda361e13fc0a70650ef82fa4ff12f5fb91a149c3feb49db395c68aa65716bc7446dedc43e94b60f994356453cf5d40a9ef676a9205ec - languageName: node - linkType: hard - -"@react-aria/selection@npm:^3.19.3": - version: 3.19.3 - resolution: "@react-aria/selection@npm:3.19.3" - dependencies: - "@react-aria/focus": ^3.18.2 - "@react-aria/i18n": ^3.12.2 - "@react-aria/interactions": ^3.22.2 - "@react-aria/utils": ^3.25.2 - "@react-stately/selection": ^3.16.2 - "@react-types/shared": ^3.24.1 - "@swc/helpers": ^0.5.0 - peerDependencies: - react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0 - react-dom: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0 - checksum: fa37f096476907cbf0583c6d544a31afa87c23b553a82b1563d36ea6bf59b19e5917423b2678ffd8961726525329b389108aff715711692cf1827cccfaff80d2 - languageName: node - linkType: hard - -"@react-aria/selection@npm:~3.18.1": - version: 3.18.1 - resolution: "@react-aria/selection@npm:3.18.1" - dependencies: - "@react-aria/focus": ^3.17.1 - "@react-aria/i18n": ^3.11.1 - "@react-aria/interactions": ^3.21.3 - "@react-aria/utils": ^3.24.1 - "@react-stately/selection": ^3.15.1 - "@react-types/shared": ^3.23.1 +"@react-aria/selection@npm:^3.18.1, @react-aria/selection@npm:^3.19.1, @react-aria/selection@npm:^3.19.3, @react-aria/selection@npm:^3.8.2, @react-aria/selection@npm:~3.21.0": + version: 3.21.0 + resolution: "@react-aria/selection@npm:3.21.0" + dependencies: + "@react-aria/focus": ^3.19.0 + "@react-aria/i18n": ^3.12.4 + "@react-aria/interactions": ^3.22.5 + "@react-aria/utils": ^3.26.0 + "@react-stately/selection": ^3.18.0 + "@react-types/shared": ^3.26.0 "@swc/helpers": ^0.5.0 peerDependencies: - react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 - react-dom: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 - checksum: 7ed29bd7ebd3a517ec1a049ad36390e0f2ccd41fbea3d826195e895e690b62fe440df96a040cbc8dcaf27eef5cb623116fc05bc995c935b45dd96ca175fd5acd + react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 + react-dom: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 + checksum: 612673bbc94b32a47788d057a020585b2416c8f4279760355b80c963efe4587f94aaf2655cbd54f8fbad0197e46fc54612a3291b945a5bd518d899e7bb46e9ae languageName: node linkType: hard @@ -6237,6 +6154,17 @@ __metadata: languageName: node linkType: hard +"@react-aria/ssr@npm:^3.9.7": + version: 3.9.7 + resolution: "@react-aria/ssr@npm:3.9.7" + dependencies: + "@swc/helpers": ^0.5.0 + peerDependencies: + react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 + checksum: 10ad277d8c4db6cf9b546f5800dd084451a4a8173a57b06c6597fd39375526a81f1fb398fe46558d372f8660d33c0a09a2580e0529351d76b2c8938482597b3f + languageName: node + linkType: hard + "@react-aria/tabs@npm:~3.1.5": version: 3.1.5 resolution: "@react-aria/tabs@npm:3.1.5" @@ -6340,6 +6268,21 @@ __metadata: languageName: node linkType: hard +"@react-aria/utils@npm:^3.26.0": + version: 3.26.0 + resolution: "@react-aria/utils@npm:3.26.0" + dependencies: + "@react-aria/ssr": ^3.9.7 + "@react-stately/utils": ^3.10.5 + "@react-types/shared": ^3.26.0 + "@swc/helpers": ^0.5.0 + clsx: ^2.0.0 + peerDependencies: + react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 + checksum: 8ad5dbfeaf41e04f6ec2b16e7f0a461614f8d0f94a1b8ce5e19a0f09a79cb49774451db485796e46ef62212ad4978c851fc645351fffbef862a48dcde9b9e1a2 + languageName: node + linkType: hard + "@react-aria/virtualizer@npm:3.3.7": version: 3.3.7 resolution: "@react-aria/virtualizer@npm:3.3.7" @@ -6399,7 +6342,7 @@ __metadata: languageName: node linkType: hard -"@react-stately/collections@npm:^3.10.7, @react-stately/collections@npm:^3.10.9, @react-stately/collections@npm:~3.10.9": +"@react-stately/collections@npm:^3.10.7, @react-stately/collections@npm:^3.10.9": version: 3.10.9 resolution: "@react-stately/collections@npm:3.10.9" dependencies: @@ -6411,6 +6354,18 @@ __metadata: languageName: node linkType: hard +"@react-stately/collections@npm:^3.12.0, @react-stately/collections@npm:~3.12.0": + version: 3.12.0 + resolution: "@react-stately/collections@npm:3.12.0" + dependencies: + "@react-types/shared": ^3.26.0 + "@swc/helpers": ^0.5.0 + peerDependencies: + react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 + checksum: 7278224dc5b7a757bcba90454afe2b03209b2ae97954782526226664918c75486ab04e418eef69c575d526135dc257125ab1b23db86a40b844dd8766bc5b3eac + languageName: node + linkType: hard + "@react-stately/combobox@npm:^3.9.2": version: 3.9.2 resolution: "@react-stately/combobox@npm:3.9.2" @@ -6599,7 +6554,7 @@ __metadata: languageName: node linkType: hard -"@react-stately/selection@npm:^3.15.1, @react-stately/selection@npm:^3.16.1": +"@react-stately/selection@npm:^3.16.1": version: 3.16.1 resolution: "@react-stately/selection@npm:3.16.1" dependencies: @@ -6627,6 +6582,20 @@ __metadata: languageName: node linkType: hard +"@react-stately/selection@npm:^3.18.0": + version: 3.18.0 + resolution: "@react-stately/selection@npm:3.18.0" + dependencies: + "@react-stately/collections": ^3.12.0 + "@react-stately/utils": ^3.10.5 + "@react-types/shared": ^3.26.0 + "@swc/helpers": ^0.5.0 + peerDependencies: + react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 + checksum: 0ae179f7b082bcc472e3ddfa585ed46d5304d1ac21c720d14c394f3030772c510318122fc095801a66b005c2174cfc7ea37298fb929a26993d73194a8bde0324 + languageName: node + linkType: hard + "@react-stately/selection@npm:~3.15.1": version: 3.15.1 resolution: "@react-stately/selection@npm:3.15.1" @@ -6747,6 +6716,17 @@ __metadata: languageName: node linkType: hard +"@react-stately/utils@npm:^3.10.5": + version: 3.10.5 + resolution: "@react-stately/utils@npm:3.10.5" + dependencies: + "@swc/helpers": ^0.5.0 + peerDependencies: + react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 + checksum: 4f4292ccf7bb86578a20b354cf9569f88d2d50ecb2e10ac6046fab3b9eb2175f734acf1b9bd87787e439220b912785a54551a724ab285f03e4f33b2942831f57 + languageName: node + linkType: hard + "@react-stately/virtualizer@npm:^3.1.7, @react-stately/virtualizer@npm:~3.1.9": version: 3.1.9 resolution: "@react-stately/virtualizer@npm:3.1.9" @@ -6883,12 +6863,12 @@ __metadata: languageName: node linkType: hard -"@react-types/shared@npm:^3.11.1, @react-types/shared@npm:^3.12.0, @react-types/shared@npm:^3.13.0, @react-types/shared@npm:^3.13.1, @react-types/shared@npm:^3.16.0, @react-types/shared@npm:^3.23.1, @react-types/shared@npm:^3.24.1, @react-types/shared@npm:~3.24.1": - version: 3.24.1 - resolution: "@react-types/shared@npm:3.24.1" +"@react-types/shared@npm:^3.11.1, @react-types/shared@npm:^3.12.0, @react-types/shared@npm:^3.13.0, @react-types/shared@npm:^3.13.1, @react-types/shared@npm:^3.16.0, @react-types/shared@npm:^3.23.1, @react-types/shared@npm:^3.24.1, @react-types/shared@npm:^3.26.0, @react-types/shared@npm:~3.26.0": + version: 3.26.0 + resolution: "@react-types/shared@npm:3.26.0" peerDependencies: - react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0 - checksum: 157ed3a210bcbdcf9aae25db5df5d0650edcc8b98686654433c9526bfb4be6431838c6480fff2710cd5b68c9a521f519d6f352e919e04bf9aed52fa0d70ed887 + react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 + checksum: f51381af98a89e1a9823ee18ed16418c5e8badd640dffb0a3523437aa003b144eea878bb49b4f62672484c361f380864d8dcaba742259da32a67b29692a63b06 languageName: node linkType: hard From c2f1fce3e97b2a3d61472df42324666c388b9999 Mon Sep 17 00:00:00 2001 From: Alireza Date: Thu, 12 Dec 2024 22:23:07 +0100 Subject: [PATCH 15/17] fix: workaround an issue in @react-aria/selection https://github.com/adobe/react-spectrum/issues/7512 --- .../jui/src/selection/useSelectableCollection.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/jui/src/selection/useSelectableCollection.ts b/packages/jui/src/selection/useSelectableCollection.ts index 7201e18d..1b8f7472 100644 --- a/packages/jui/src/selection/useSelectableCollection.ts +++ b/packages/jui/src/selection/useSelectableCollection.ts @@ -21,7 +21,19 @@ export const useSelectableCollection: typeof useAriaSelectableCollection = ( ) { selectionManager.replaceSelection(selectionManager.focusedKey); } - }); + + // Working around https://github.com/adobe/react-spectrum/issues/7512 + // FIXME: remove the workaround the issue is closed + if ( + selectionManager.firstSelectedKey && + selectionManager.focusedKey == null + ) { + // initialize the focusedKey so that the buggy code that mutates selection + // onFocus doesn't get to run. + selectionManager.setFocusedKey(selectionManager.firstSelectedKey); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); return { collectionProps: { ...collectionProps, From c30fdd54a3cc8a6a7a07a1e37ca4cddac016c51f Mon Sep 17 00:00:00 2001 From: Alireza Date: Thu, 12 Dec 2024 23:03:50 +0100 Subject: [PATCH 16/17] test: working around the failure of a test case in CI env Trying a little wait between finding the menu in the dom and scrolling, just in case the scroll handlers inside menu overlay are set up with a tiny delay. Doesn't seem that plausible though. --- packages/jui/src/Menu/Menu.cy.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/jui/src/Menu/Menu.cy.tsx b/packages/jui/src/Menu/Menu.cy.tsx index 5a39378f..9a260b92 100644 --- a/packages/jui/src/Menu/Menu.cy.tsx +++ b/packages/jui/src/Menu/Menu.cy.tsx @@ -922,6 +922,12 @@ describe("ContextMenu", () => { scrollBehavior: false, }); cy.findByRole("menu"); + + // onScroll was called in a run of the pipeline on the CI env. + // Not sure why but trying to work around the flakiness by a wait. + // https://cloud.cypress.io/projects/o1ooqz/runs/255/overview/0aa04a42-1030-4ab6-a8b2-1fe08069a705?roarHideRunsWithDiffGroupsAndTags=1 + cy.wait(100); + cy.get("#container").realMouseWheel({ deltaY: 5, scrollBehavior: false, // we don't want cypress to do any extra scrolling From b3123b75d1d970bf1738acd5db8c14fd47c16bbc Mon Sep 17 00:00:00 2001 From: Alireza Date: Thu, 12 Dec 2024 23:32:52 +0100 Subject: [PATCH 17/17] test: refactor a menu test case to have it pass on CI env For some unknown reason, scroll events were triggered upon mouse wheel, even though scroll was prevented. It didn't happen when running locally headless with `cypress run`, so it seems like the only differentiating factor is the OS being linux in the CI env. The test is changed to compare actual scrollY of the window, instead of checking if scroll event handlers are called. --- packages/jui/src/Menu/Menu.cy.tsx | 51 +++++++++++++------------------ 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/packages/jui/src/Menu/Menu.cy.tsx b/packages/jui/src/Menu/Menu.cy.tsx index 9a260b92..b6564ca2 100644 --- a/packages/jui/src/Menu/Menu.cy.tsx +++ b/packages/jui/src/Menu/Menu.cy.tsx @@ -889,51 +889,44 @@ describe("ContextMenu", () => { }); it("disables the scroll of the document when the menu is open", () => { - const ScrollListener = (props: { onScroll: () => void }) => { - React.useEffect(() => { - const onScroll = () => { - props.onScroll(); - }; - document.addEventListener("scroll", onScroll); - return () => { - document.removeEventListener("scroll", onScroll); - }; - }); - return null; - }; - const onScroll = cy.stub().as("onScroll"); cy.mount( <> - +
( Menu item )} > - {" "} + scrollable container +
); cy.get("#container").rightclick("top", { scrollBehavior: false, }); - cy.findByRole("menu"); - - // onScroll was called in a run of the pipeline on the CI env. - // Not sure why but trying to work around the flakiness by a wait. - // https://cloud.cypress.io/projects/o1ooqz/runs/255/overview/0aa04a42-1030-4ab6-a8b2-1fe08069a705?roarHideRunsWithDiffGroupsAndTags=1 - cy.wait(100); - - cy.get("#container").realMouseWheel({ - deltaY: 5, - scrollBehavior: false, // we don't want cypress to do any extra scrolling - }); - cy.wait(200); // maybe a bug in realMouseWheel, but before the wait, the next command runs before scrolling happens - cy.wrap(onScroll).should("not.be.called"); + cy.window() + .its("scrollY") + .then((scrollBefore) => { + cy.get("#element-above").realMouseWheel({ + deltaY: 5, + scrollBehavior: false, // we don't want cypress to do any extra scrolling + }); + cy.wait(200); // maybe a bug in realMouseWheel, but before the wait, the next command runs before scrolling happens + cy.window() + .its("scrollY") + .then((scrollAfter) => { + expect(scrollAfter).to.eq(scrollBefore); + }); + }); }); it("disables the scroll when the menu is open", () => {