From cf3dab13ad673cdebc61770c32376f2ac8e9a14d Mon Sep 17 00:00:00 2001 From: haykam821 <24855774+haykam821@users.noreply.github.com> Date: Wed, 24 Aug 2022 23:14:59 -0400 Subject: [PATCH 01/23] Include renamed files in the commit summary changed files tooltip Fixes #15155 --- app/src/ui/history/commit-summary.tsx | 12 ++++++++++++ app/styles/ui/window/_tooltips.scss | 4 ++++ 2 files changed, 16 insertions(+) diff --git a/app/src/ui/history/commit-summary.tsx b/app/src/ui/history/commit-summary.tsx index c8853a9086..82f0b80e78 100644 --- a/app/src/ui/history/commit-summary.tsx +++ b/app/src/ui/history/commit-summary.tsx @@ -560,6 +560,7 @@ export class CommitSummary extends React.Component< let filesAdded = 0 let filesModified = 0 let filesRemoved = 0 + let filesRenamed = 0 for (const file of this.props.changesetData.files) { switch (file.status.kind) { case AppFileStatusKind.New: @@ -571,6 +572,8 @@ export class CommitSummary extends React.Component< case AppFileStatusKind.Deleted: filesRemoved += 1 break + case AppFileStatusKind.Renamed: + filesRenamed += 1 } } @@ -603,6 +606,15 @@ export class CommitSummary extends React.Component< {filesRemoved} deleted ) : null} + {filesRenamed > 0 ? ( + + + {filesRenamed} renamed + + ) : null} ) diff --git a/app/styles/ui/window/_tooltips.scss b/app/styles/ui/window/_tooltips.scss index daed043727..9d3326af33 100644 --- a/app/styles/ui/window/_tooltips.scss +++ b/app/styles/ui/window/_tooltips.scss @@ -192,6 +192,10 @@ body > .tooltip, color: var(--color-deleted); } + .files-renamed-icon { + color: var(--color-renamed); + } + .octicon { margin-right: var(--spacing-third); vertical-align: bottom; // For some reason, `bottom` places the text in the middle From bf9cdf18ca3f9d375f011ed69c91a7d1ba0c3454 Mon Sep 17 00:00:00 2001 From: jongwooo Date: Sat, 7 Jan 2023 00:44:43 +0900 Subject: [PATCH 02/23] Use setup-node action to cache dependencies Signed-off-by: jongwooo --- .github/workflows/ci.yml | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b8c8771aff..862056b7d5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,19 +32,7 @@ jobs: uses: actions/setup-node@v3 with: node-version: ${{ matrix.node }} - - - name: Get yarn cache directory path - id: yarn-cache-dir-path - run: echo "::set-output name=dir::$(yarn cache dir)" - - - uses: actions/cache@v3 - id: yarn-cache # use this to check for `cache-hit` (`steps.yarn-cache.outputs.cache-hit != 'true'`) - with: - path: ${{ steps.yarn-cache-dir-path.outputs.dir }} - key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }} - restore-keys: | - ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }} - ${{ runner.os }}-yarn- + cache: yarn # This step can be removed as soon as official Windows arm64 builds are published: # https://github.com/nodejs/build/issues/2450#issuecomment-705853342 From 5fd16c922bc5c628b6897d47ad2c318e1f0ac9ec Mon Sep 17 00:00:00 2001 From: Jorge Rivera Date: Fri, 27 Jan 2023 14:48:20 +0100 Subject: [PATCH 03/23] Add DataSpell to darwin.ts Modify list of external editors to include JetBrains DataSpell --- app/src/lib/editors/darwin.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/src/lib/editors/darwin.ts b/app/src/lib/editors/darwin.ts index 7634e7482e..76c475145e 100644 --- a/app/src/lib/editors/darwin.ts +++ b/app/src/lib/editors/darwin.ts @@ -71,6 +71,10 @@ const editors: IDarwinExternalEditor[] = [ name: 'PyCharm Community Edition', bundleIdentifiers: ['com.jetbrains.pycharm.ce'], }, + { + name: 'DataSpell', + bundleIdentifiers: ['com.jetbrains.DataSpell'], + }, { name: 'RubyMine', bundleIdentifiers: ['com.jetbrains.RubyMine'], From 13332ca8e300224162599dc855e919231499636d Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Fri, 27 Jan 2023 10:17:15 -0500 Subject: [PATCH 04/23] Check for content --- app/src/ui/history/commit-summary.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/src/ui/history/commit-summary.tsx b/app/src/ui/history/commit-summary.tsx index 3799d96262..01f59cc0a1 100644 --- a/app/src/ui/history/commit-summary.tsx +++ b/app/src/ui/history/commit-summary.tsx @@ -553,6 +553,9 @@ export class CommitSummary extends React.Component< } } + const hasFileDescription = + filesAdded + filesModified + filesRemoved + filesRemoved > 0 + const filesLongDescription = ( <> {filesAdded > 0 ? ( @@ -598,7 +601,9 @@ export class CommitSummary extends React.Component< 0 ? filesLongDescription : undefined} + tooltip={ + fileCount > 0 && hasFileDescription ? filesLongDescription : undefined + } > {filesShortDescription} From 63592700cf311b34ec5b3fd711f2099412d0b8d2 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Fri, 27 Jan 2023 10:52:12 -0500 Subject: [PATCH 05/23] Need each one.. Co-authored-by: Sergio Padrino --- app/src/ui/history/commit-summary.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/ui/history/commit-summary.tsx b/app/src/ui/history/commit-summary.tsx index 01f59cc0a1..e342225d71 100644 --- a/app/src/ui/history/commit-summary.tsx +++ b/app/src/ui/history/commit-summary.tsx @@ -554,7 +554,7 @@ export class CommitSummary extends React.Component< } const hasFileDescription = - filesAdded + filesModified + filesRemoved + filesRemoved > 0 + filesAdded + filesModified + filesRemoved + filesRenamed > 0 const filesLongDescription = ( <> From 7bb7724f60abb1626c324c647dfb0a5092e868d1 Mon Sep 17 00:00:00 2001 From: Adil Hanney Date: Sun, 29 Jan 2023 20:24:39 +0000 Subject: [PATCH 06/23] Add some IDEs from JetBrains Toolbox to Linux I'm not sure about the snap locations but I've added them in for completeness. If I should remove them, let me know :) --- app/src/lib/editors/linux.ts | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/app/src/lib/editors/linux.ts b/app/src/lib/editors/linux.ts index 0670548b44..93228c350b 100644 --- a/app/src/lib/editors/linux.ts +++ b/app/src/lib/editors/linux.ts @@ -63,12 +63,24 @@ const editors: ILinuxExternalEditor[] = [ paths: ['/usr/bin/lite-xl'], }, { - name: 'Jetbrains PhpStorm', - paths: ['/snap/bin/phpstorm'], + name: 'JetBrains PhpStorm', + paths: ['/snap/bin/phpstorm', '.local/share/JetBrains/Toolbox/scripts/phpstorm'], }, { - name: 'Jetbrains WebStorm', - paths: ['/snap/bin/webstorm'], + name: 'JetBrains WebStorm', + paths: ['/snap/bin/webstorm', '.local/share/JetBrains/Toolbox/scripts/webstorm'], + }, + { + name: 'IntelliJ IDEA', + paths: ['/snap/bin/idea', '.local/share/JetBrains/Toolbox/scripts/idea'], + }, + { + name: 'Jetbrains PyCharm', + paths: ['/snap/bin/pycharm', '.local/share/JetBrains/Toolbox/scripts/pycharm'], + }, + { + name: 'Android Studio', + paths: ['/snap/bin/studio', '.local/share/JetBrains/Toolbox/scripts/studio'], }, { name: 'Emacs', From 5f777061203c6ac326fd9b267ef52a24e5ed36f8 Mon Sep 17 00:00:00 2001 From: Adil Hanney Date: Sun, 29 Jan 2023 20:30:03 +0000 Subject: [PATCH 07/23] Recapitalise JetBrains --- app/src/lib/editors/linux.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/lib/editors/linux.ts b/app/src/lib/editors/linux.ts index 93228c350b..6ab8a1e653 100644 --- a/app/src/lib/editors/linux.ts +++ b/app/src/lib/editors/linux.ts @@ -75,7 +75,7 @@ const editors: ILinuxExternalEditor[] = [ paths: ['/snap/bin/idea', '.local/share/JetBrains/Toolbox/scripts/idea'], }, { - name: 'Jetbrains PyCharm', + name: 'JetBrains PyCharm', paths: ['/snap/bin/pycharm', '.local/share/JetBrains/Toolbox/scripts/pycharm'], }, { From b37a25a88e3cf140337ad6879609c82a28d427c6 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Sun, 29 Jan 2023 16:36:00 -0400 Subject: [PATCH 08/23] Add support for numerous additional editors on Linux - Kate - gEdit - Notepadqq - GNOME Text Editor - GNOME Builder - VSCode (from WSL) - Geany - Mousepad Co-Authored-By: gwyn <29720696+avalonv@users.noreply.github.com> Co-Authored-By: PadowYT2 <71085027+PadowYT2@users.noreply.github.com> Co-Authored-By: theofficialgman <28281419+theofficialgman@users.noreply.github.com> Co-Authored-By: Amin Yahyaabadi <16418197+aminya@users.noreply.github.com> Co-Authored-By: Etaash Mathamsetty <45927311+Etaash-mathamsetty@users.noreply.github.com> Co-Authored-By: Richard Kellnberger <5147221+Richardk2n@users.noreply.github.com> Co-Authored-By: Sky Barnes <429099+tsbarnes@users.noreply.github.com> --- app/src/lib/editors/linux.ts | 53 ++++++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/app/src/lib/editors/linux.ts b/app/src/lib/editors/linux.ts index 0670548b44..8136ac932c 100644 --- a/app/src/lib/editors/linux.ts +++ b/app/src/lib/editors/linux.ts @@ -23,9 +23,26 @@ const editors: ILinuxExternalEditor[] = [ name: 'Neovim', paths: ['/usr/bin/nvim'], }, + { + name: 'Neovim-Qt', + paths: ['/usr/bin/nvim-qt'], + }, + { + name: 'Neovide', + paths: ['/usr/bin/neovide'], + }, + { + name: 'gVim', + paths: ['/usr/bin/gvim'], + }, { name: 'Visual Studio Code', - paths: ['/usr/share/code/bin/code', '/snap/bin/code', '/usr/bin/code'], + paths: [ + '/usr/share/code/bin/code', + '/snap/bin/code', + '/usr/bin/code', + '/mnt/c/Program Files/Microsoft VS Code/bin/code', + ], }, { name: 'Visual Studio Code (Insiders)', @@ -33,7 +50,11 @@ const editors: ILinuxExternalEditor[] = [ }, { name: 'VSCodium', - paths: ['/usr/bin/codium', '/var/lib/flatpak/app/com.vscodium.codium'], + paths: [ + '/usr/bin/codium', + '/var/lib/flatpak/app/com.vscodium.codium', + '/usr/share/vscodium-bin/bin/codium', + ], }, { name: 'Sublime Text', @@ -74,6 +95,34 @@ const editors: ILinuxExternalEditor[] = [ name: 'Emacs', paths: ['/snap/bin/emacs', '/usr/local/bin/emacs', '/usr/bin/emacs'], }, + { + name: 'Kate', + paths: ['/usr/bin/kate'], + }, + { + name: 'GEdit', + paths: ['/usr/bin/gedit'], + }, + { + name: 'GNOME Text Editor', + paths: ['/usr/bin/gnome-text-editor'], + }, + { + name: 'GNOME Builder', + paths: ['/usr/bin/gnome-builder'], + }, + { + name: 'Notepadqq', + paths: ['/usr/bin/notepadqq'], + }, + { + name: 'Geany', + paths: ['/usr/bin/geany'], + }, + { + name: 'Mousepad', + paths: ['/usr/bin/mousepad'], + }, ] async function getAvailablePath(paths: string[]): Promise { From 47152957bf8ca79115942f359ca7105cf365d818 Mon Sep 17 00:00:00 2001 From: Tsvetilian Yankov Date: Mon, 30 Jan 2023 00:43:17 +0200 Subject: [PATCH 09/23] Add support for JetBrains DataSpell on Windows --- app/src/lib/editors/win32.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/src/lib/editors/win32.ts b/app/src/lib/editors/win32.ts index afdc6c09ba..90ac080f7f 100644 --- a/app/src/lib/editors/win32.ts +++ b/app/src/lib/editors/win32.ts @@ -471,6 +471,14 @@ const editors: WindowsExternalEditor[] = [ displayNamePrefix: 'Fleet ', publishers: ['JetBrains s.r.o.'], }, + { + name: 'JetBrains DataSpell', + registryKeys: registryKeysForJetBrainsIDE('DataSpell'), + executableShimPaths: executableShimPathsForJetBrainsIDE('dataspell'), + jetBrainsToolboxScriptName: 'dataspell', + displayNamePrefix: 'DataSpell ', + publishers: ['JetBrains s.r.o.'], + }, ] function getKeyOrEmpty( From 3c211d1a40d865a52d81e18ac8166bfc78c5a0ca Mon Sep 17 00:00:00 2001 From: Tsvetilian Yankov Date: Mon, 30 Jan 2023 00:45:22 +0200 Subject: [PATCH 10/23] Update supported editors list --- docs/technical/editor-integration.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/technical/editor-integration.md b/docs/technical/editor-integration.md index 6cdb0386d4..ddba8b72b6 100644 --- a/docs/technical/editor-integration.md +++ b/docs/technical/editor-integration.md @@ -45,6 +45,7 @@ These editors are currently supported: - [RStudio](https://rstudio.com/) - [Aptana Studio](http://www.aptana.com/) - [JetBrains Fleet](https://www.jetbrains.com/fleet/) + - [JetBrains DataSpell](https://www.jetbrains.com/dataspell/) These are defined in a list at the top of the file: @@ -272,6 +273,7 @@ These editors are currently supported: - [Emacs](https://www.gnu.org/software/emacs/) - [Lite XL](https://lite-xl.com/) - [JetBrains Fleet](https://www.jetbrains.com/fleet/) + - [JetBrains DataSpell](https://www.jetbrains.com/dataspell/) These are defined in a list at the top of the file: From 89c4daa00a40d054513ea6fb65929c639dfc7b8f Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 1 Feb 2023 13:03:30 -0500 Subject: [PATCH 11/23] Add axeDevTools extension --- app/src/main-process/main.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/src/main-process/main.ts b/app/src/main-process/main.ts index 75a889f5f6..bef7e254ea 100644 --- a/app/src/main-process/main.ts +++ b/app/src/main-process/main.ts @@ -729,7 +729,13 @@ function createWindow() { electron: '>=1.2.1', } - const extensions = [REACT_DEVELOPER_TOOLS, ChromeLens] + const axeDevTools = { + id: 'lhdoppojpmngadmnindnejefpokejbdd', + electron: '>=1.2.1', + Permissions: ['tabs', 'debugger'], + } + + const extensions = [REACT_DEVELOPER_TOOLS, ChromeLens, axeDevTools] for (const extension of extensions) { try { From fe3fdfa62d27471d95bd393ce04581066eb4f709 Mon Sep 17 00:00:00 2001 From: Adil Hanney Date: Thu, 2 Feb 2023 12:54:05 +0000 Subject: [PATCH 12/23] `yarn lint:fix` --- app/src/lib/editors/linux.ts | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/app/src/lib/editors/linux.ts b/app/src/lib/editors/linux.ts index 6ab8a1e653..37868ed44f 100644 --- a/app/src/lib/editors/linux.ts +++ b/app/src/lib/editors/linux.ts @@ -64,11 +64,17 @@ const editors: ILinuxExternalEditor[] = [ }, { name: 'JetBrains PhpStorm', - paths: ['/snap/bin/phpstorm', '.local/share/JetBrains/Toolbox/scripts/phpstorm'], + paths: [ + '/snap/bin/phpstorm', + '.local/share/JetBrains/Toolbox/scripts/phpstorm', + ], }, { name: 'JetBrains WebStorm', - paths: ['/snap/bin/webstorm', '.local/share/JetBrains/Toolbox/scripts/webstorm'], + paths: [ + '/snap/bin/webstorm', + '.local/share/JetBrains/Toolbox/scripts/webstorm', + ], }, { name: 'IntelliJ IDEA', @@ -76,11 +82,17 @@ const editors: ILinuxExternalEditor[] = [ }, { name: 'JetBrains PyCharm', - paths: ['/snap/bin/pycharm', '.local/share/JetBrains/Toolbox/scripts/pycharm'], + paths: [ + '/snap/bin/pycharm', + '.local/share/JetBrains/Toolbox/scripts/pycharm', + ], }, { name: 'Android Studio', - paths: ['/snap/bin/studio', '.local/share/JetBrains/Toolbox/scripts/studio'], + paths: [ + '/snap/bin/studio', + '.local/share/JetBrains/Toolbox/scripts/studio', + ], }, { name: 'Emacs', From 421f7835b6f34f762a147fa33fb7e0697b17f374 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 2 Feb 2023 17:33:28 +0000 Subject: [PATCH 13/23] Bump jszip from 3.7.1 to 3.8.0 Bumps [jszip](https://github.com/Stuk/jszip) from 3.7.1 to 3.8.0. - [Release notes](https://github.com/Stuk/jszip/releases) - [Changelog](https://github.com/Stuk/jszip/blob/main/CHANGES.md) - [Commits](https://github.com/Stuk/jszip/compare/v3.7.1...v3.8.0) --- updated-dependencies: - dependency-name: jszip dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 5e0e53586b..e5ac739268 100644 --- a/package.json +++ b/package.json @@ -85,7 +85,7 @@ "jest-diff": "^25.0.0", "jest-extended": "^0.11.2", "jest-localstorage-mock": "^2.3.0", - "jszip": "^3.7.1", + "jszip": "^3.8.0", "klaw-sync": "^3.0.0", "legal-eagle": "0.16.0", "mini-css-extract-plugin": "^2.5.3", diff --git a/yarn.lock b/yarn.lock index a9dab1caa2..a47efb4319 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6558,10 +6558,10 @@ jsx-ast-utils@^3.3.2: array-includes "^3.1.5" object.assign "^4.1.3" -jszip@^3.7.1: - version "3.7.1" - resolved "https://registry.yarnpkg.com/jszip/-/jszip-3.7.1.tgz#bd63401221c15625a1228c556ca8a68da6fda3d9" - integrity sha512-ghL0tz1XG9ZEmRMcEN2vt7xabrDdqHHeykgARpmZ0BiIctWxM47Vt63ZO2dnp4QYt/xJVLLy5Zv1l/xRdh2byg== +jszip@^3.8.0: + version "3.8.0" + resolved "https://registry.yarnpkg.com/jszip/-/jszip-3.8.0.tgz#a2ac3c33fe96a76489765168213655850254d51b" + integrity sha512-cnpQrXvFSLdsR9KR5/x7zdf6c3m8IhZfZzSblFEHSqBaVwD2nvJ4CuCKLyvKvwBgZm08CgfSoiTBQLm5WW9hGw== dependencies: lie "~3.3.0" pako "~1.0.2" From 3a0af15c24bcabd1ec41a13071ceac6eef0199c1 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 3 Feb 2023 05:13:06 +0000 Subject: [PATCH 14/23] Bump http-cache-semantics from 4.1.0 to 4.1.1 Bumps [http-cache-semantics](https://github.com/kornelski/http-cache-semantics) from 4.1.0 to 4.1.1. - [Release notes](https://github.com/kornelski/http-cache-semantics/releases) - [Commits](https://github.com/kornelski/http-cache-semantics/compare/v4.1.0...v4.1.1) --- updated-dependencies: - dependency-name: http-cache-semantics dependency-type: indirect ... Signed-off-by: dependabot[bot] --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index a9dab1caa2..ea0d09b0f6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5245,9 +5245,9 @@ htmlparser2@^6.1.0: entities "^2.0.0" http-cache-semantics@^4.0.0: - version "4.1.0" - resolved "https://registry.yarnpkg.com/http-cache-semantics/-/http-cache-semantics-4.1.0.tgz#49e91c5cbf36c9b94bcfcd71c23d5249ec74e390" - integrity sha512-carPklcUh7ROWRK7Cv27RPtdhYhUsela/ue5/jKzjegVvXDqM2ILE9Q2BGn9JZJh1g87cp56su/FgQSzcWS8cQ== + version "4.1.1" + resolved "https://registry.yarnpkg.com/http-cache-semantics/-/http-cache-semantics-4.1.1.tgz#abe02fcb2985460bf0323be664436ec3476a6d5a" + integrity sha512-er295DKPVsV82j5kw1Gjt+ADA/XYHsajl82cGNQG2eyoPkvgUhX+nDIyelzhIWbbsXP39EHcI6l5tYs2FYqYXQ== http-errors@1.8.1: version "1.8.1" From 43f1b55d8e34a93881468559c392dd4a155d43e5 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Wed, 25 Jan 2023 18:08:23 +0100 Subject: [PATCH 15/23] Merge pull request #15915 from desktop/bump-git-2.39.0 Bump to git 2.39.0 and git-lfs 3.3.0 --- app/package.json | 2 +- app/src/lib/git/diff.ts | 2 +- app/yarn.lock | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/package.json b/app/package.json index 7826932d4f..7fe8d901eb 100644 --- a/app/package.json +++ b/app/package.json @@ -30,7 +30,7 @@ "desktop-trampoline": "desktop/desktop-trampoline#v0.9.8", "dexie": "^3.2.2", "dompurify": "^2.3.3", - "dugite": "^2.2.0", + "dugite": "^2.3.0", "electron-window-state": "^5.0.3", "event-kit": "^2.0.0", "focus-trap-react": "^8.1.0", diff --git a/app/src/lib/git/diff.ts b/app/src/lib/git/diff.ts index 0f7f0fcb4d..c43cf46278 100644 --- a/app/src/lib/git/diff.ts +++ b/app/src/lib/git/diff.ts @@ -530,7 +530,7 @@ function getMediaType(extension: string) { * changes based on what the user has configured. */ const lineEndingsChangeRegex = - /warning: (CRLF|CR|LF) will be replaced by (CRLF|CR|LF) in .*/ + /', (CRLF|CR|LF) will be replaced by (CRLF|CR|LF) the .*/ /** * Utility function for inspecting the stderr output for the line endings diff --git a/app/yarn.lock b/app/yarn.lock index c7432790a6..e9b374020d 100644 --- a/app/yarn.lock +++ b/app/yarn.lock @@ -387,10 +387,10 @@ dompurify@^2.3.3: resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-2.3.3.tgz#c1af3eb88be47324432964d8abc75cf4b98d634c" integrity sha512-dqnqRkPMAjOZE0FogZ+ceJNM2dZ3V/yNOuFB7+39qpO93hHhfRpHw3heYQC7DPK9FqbQTfBKUJhiSfz4MvXYwg== -dugite@^2.2.0: - version "2.2.0" - resolved "https://registry.yarnpkg.com/dugite/-/dugite-2.2.0.tgz#6678a463acd2e3e489736555eb2baf4224d087db" - integrity sha512-cCzmtNj0UNglm94gCgw0KkEuzlVp2DhRJ/rwyQVXju/jFQTE1WWBVe5YCSanCIUSyu80E1JqQvCvQ9ywcmRb1Q== +dugite@^2.3.0: + version "2.3.0" + resolved "https://registry.yarnpkg.com/dugite/-/dugite-2.3.0.tgz#ff6fdb4c899f84ed6695c9e01eaf4364a6211f13" + integrity sha512-78zuD3p5lx2IS8DilVvHbXQXRo+hGIb3EAshTEC3ZyBLyArKegA8R/6c4Ne1aUlx6JRf3wmKNgYkdJOYMyj9aA== dependencies: progress "^2.0.3" tar "^6.1.11" From 3abd7b29ac5b84095a2fcc0e5a8537a197652ac9 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Mon, 6 Feb 2023 16:50:50 +0100 Subject: [PATCH 16/23] Bump changelog and version to 3.1.6 --- app/package.json | 2 +- changelog.json | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/package.json b/app/package.json index 7fe8d901eb..d5a6f8a25c 100644 --- a/app/package.json +++ b/app/package.json @@ -3,7 +3,7 @@ "productName": "GitHub Desktop", "bundleID": "com.github.GitHubClient", "companyName": "GitHub, Inc.", - "version": "3.1.5", + "version": "3.1.6", "main": "./main.js", "repository": { "type": "git", diff --git a/changelog.json b/changelog.json index c7436fcea4..7bc754c45c 100644 --- a/changelog.json +++ b/changelog.json @@ -1,5 +1,8 @@ { "releases": { + "3.1.6": [ + "[Improved] Upgrade embedded Git to 2.39.1 and Git LFS to 3.3.0 - #15915" + ], "3.1.5": [ "[Added] Enable menu option to Force-push branches that have diverged - #15211", "[Added] Add menu option to Fetch the current repository at any time - #7805", From bae2b7dc5c1dd63d9f7d90375498164c8b5428d6 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Thu, 9 Feb 2023 13:14:12 +0100 Subject: [PATCH 17/23] Refactor PR review dialog --- app/src/lib/stores/alive-store.ts | 1 - app/src/lib/stores/app-store.ts | 4 +- app/src/lib/stores/notifications-store.ts | 10 +- app/src/models/popup.ts | 1 - app/src/ui/app.tsx | 3 +- .../pull-request-comment-like.tsx | 200 ++++++++++++++++++ .../ui/notifications/pull-request-review.tsx | 195 +++-------------- app/styles/_ui.scss | 1 - app/styles/ui/_dialog.scss | 1 + .../_pull-request-comment-like.scss} | 9 +- 10 files changed, 242 insertions(+), 183 deletions(-) create mode 100644 app/src/ui/notifications/pull-request-comment-like.tsx rename app/styles/ui/{_pull-request-review.scss => dialogs/_pull-request-comment-like.scss} (96%) diff --git a/app/src/lib/stores/alive-store.ts b/app/src/lib/stores/alive-store.ts index e8639ee2d4..73b7a73dda 100644 --- a/app/src/lib/stores/alive-store.ts +++ b/app/src/lib/stores/alive-store.ts @@ -28,7 +28,6 @@ export interface IDesktopPullRequestReviewSubmitAliveEvent { readonly pull_request_number: number readonly state: 'APPROVED' | 'CHANGES_REQUESTED' | 'COMMENTED' readonly review_id: string - readonly number_of_comments: number } /** Represents an Alive event relevant to Desktop. */ diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index 6a2e8b5260..e25b741eb1 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -7300,8 +7300,7 @@ export class AppStore extends TypedBaseStore { private onPullRequestReviewSubmitNotification = async ( repository: RepositoryWithGitHubRepository, pullRequest: PullRequest, - review: ValidNotificationPullRequestReview, - numberOfComments: number + review: ValidNotificationPullRequestReview ) => { const selectedRepository = this.selectedRepository ?? (await this._selectRepository(repository)) @@ -7322,7 +7321,6 @@ export class AppStore extends TypedBaseStore { review, pullRequest, repository, - numberOfComments, }) } diff --git a/app/src/lib/stores/notifications-store.ts b/app/src/lib/stores/notifications-store.ts index b5c61ab29d..fc3a37f88e 100644 --- a/app/src/lib/stores/notifications-store.ts +++ b/app/src/lib/stores/notifications-store.ts @@ -48,8 +48,7 @@ type OnChecksFailedCallback = ( type OnPullRequestReviewSubmitCallback = ( repository: RepositoryWithGitHubRepository, pullRequest: PullRequest, - review: ValidNotificationPullRequestReview, - numberOfComments: number + review: ValidNotificationPullRequestReview ) => void /** @@ -175,12 +174,7 @@ export class NotificationsStore { const onClick = () => { this.statsStore.recordPullRequestReviewNotificationClicked(review.state) - this.onPullRequestReviewSubmitCallback?.( - repository, - pullRequest, - review, - event.number_of_comments - ) + this.onPullRequestReviewSubmitCallback?.(repository, pullRequest, review) } if (skipNotification) { diff --git a/app/src/models/popup.ts b/app/src/models/popup.ts index 7b21a8ca24..96d9a16f2a 100644 --- a/app/src/models/popup.ts +++ b/app/src/models/popup.ts @@ -361,7 +361,6 @@ export type PopupDetail = repository: RepositoryWithGitHubRepository pullRequest: PullRequest review: ValidNotificationPullRequestReview - numberOfComments: number shouldCheckoutBranch: boolean shouldChangeRepository: boolean } diff --git a/app/src/ui/app.tsx b/app/src/ui/app.tsx index bbe2be7374..0fb3f95fd0 100644 --- a/app/src/ui/app.tsx +++ b/app/src/ui/app.tsx @@ -2262,14 +2262,13 @@ export class App extends React.Component { case PopupType.PullRequestReview: { return ( + readonly repository: RepositoryWithGitHubRepository + readonly pullRequest: PullRequest + readonly eventDate: Date + readonly eventVerb: string + readonly eventIconSymbol: OcticonSymbolType + readonly eventIconClass: string + readonly externalURL: string + readonly user: IAPIIdentity + readonly body: string + + /** Map from the emoji shortcut (e.g., :+1:) to the image's local path. */ + readonly emoji: Map + + readonly switchingToPullRequest: boolean + + readonly renderFooterContent: () => JSX.Element + + readonly onSubmit: () => void + readonly onDismissed: () => void +} + +/** + * Dialog to show a pull request review. + */ +export abstract class PullRequestCommentLike extends React.Component { + public render() { + const { title, pullRequestNumber } = this.props.pullRequest + + const header = ( +
+ {this.renderPullRequestIcon()} + + {title}{' '} + #{pullRequestNumber}{' '} + +
+ ) + + return ( + + +
+ {this.renderTimelineItem()} + {this.renderCommentBubble()} +
+ + {this.props.renderFooterContent()} + + ) + } + + private renderTimelineItem() { + const { user, repository, eventDate, eventVerb, externalURL } = this.props + const { endpoint } = repository.gitHubRepository + const userAvatar = { + name: user.login, + email: getStealthEmailForUser(user.id, user.login, endpoint), + avatarURL: user.avatar_url, + endpoint: endpoint, + } + + const bottomLine = this.shouldRenderCommentBubble() + ? null + : this.renderDashedTimelineLine('bottom') + + const timelineItemClass = classNames('timeline-item', { + 'with-comment': this.shouldRenderCommentBubble(), + }) + + const diff = eventDate.getTime() - Date.now() + const relativeReviewDate = formatRelative(diff) + + return ( +
+ {this.renderDashedTimelineLine('top')} +
+ + {this.renderReviewIcon()} +
+ + {user.login} + {' '} + {eventVerb} your pull request{' '} + + {relativeReviewDate} + +
+
+ {bottomLine} +
+ ) + } + + private shouldRenderCommentBubble() { + return this.props.body !== '' + } + + private renderCommentBubble() { + if (!this.shouldRenderCommentBubble()) { + return null + } + + return ( +
+
{this.renderReviewBody()}
+ {this.renderDashedTimelineLine('bottom')} +
+ ) + } + + private renderDashedTimelineLine(type: 'top' | 'bottom') { + return ( + + {/* Need to use 0.5 for X to prevent nearest neighbour filtering causing + the line to appear semi-transparent. */} + + + ) + } + + private onMarkdownLinkClicked = (url: string) => { + this.props.dispatcher.openInBrowser(url) + } + + private renderReviewBody() { + const { body, emoji, pullRequest } = this.props + const { base } = pullRequest + + return ( + + ) + } + + private renderPullRequestIcon = () => { + const { pullRequest } = this.props + + const cls = classNames('pull-request-icon', { + draft: pullRequest.draft, + }) + + return ( + + ) + } + + private renderReviewIcon = () => { + const { eventIconSymbol, eventIconClass } = this.props + + return ( +
+ +
+ ) + } +} diff --git a/app/src/ui/notifications/pull-request-review.tsx b/app/src/ui/notifications/pull-request-review.tsx index d98303a76b..d0714aae50 100644 --- a/app/src/ui/notifications/pull-request-review.tsx +++ b/app/src/ui/notifications/pull-request-review.tsx @@ -1,24 +1,17 @@ import * as React from 'react' -import { Dialog, DialogContent, DialogFooter } from '../dialog' import { Row } from '../lib/row' import { OkCancelButtonGroup } from '../dialog/ok-cancel-button-group' import { PullRequest } from '../../models/pull-request' import { Dispatcher } from '../dispatcher' import { Account } from '../../models/account' -import { Octicon } from '../octicons' -import * as OcticonSymbol from '../octicons/octicons.generated' import { RepositoryWithGitHubRepository } from '../../models/repository' -import { SandboxedMarkdown } from '../lib/sandboxed-markdown' import { getPullRequestReviewStateIcon, getVerbForPullRequestReview, } from './pull-request-review-helpers' import { LinkButton } from '../lib/link-button' -import classNames from 'classnames' -import { Avatar } from '../lib/avatar' -import { formatRelative } from '../../lib/format-relative' import { ValidNotificationPullRequestReview } from '../../lib/valid-notification-pull-request-review' -import { getStealthEmailForUser } from '../../lib/email' +import { PullRequestCommentLike } from './pull-request-comment-like' interface IPullRequestReviewProps { readonly dispatcher: Dispatcher @@ -26,7 +19,6 @@ interface IPullRequestReviewProps { readonly repository: RepositoryWithGitHubRepository readonly pullRequest: PullRequest readonly review: ValidNotificationPullRequestReview - readonly numberOfComments: number /** Map from the emoji shortcut (e.g., :+1:) to the image's local path. */ readonly emoji: Map @@ -47,7 +39,7 @@ interface IPullRequestReviewState { } /** - * Dialog to show the result of a CI check run. + * Dialog to show a pull request review. */ export class PullRequestReview extends React.Component< IPullRequestReviewProps, @@ -62,115 +54,42 @@ export class PullRequestReview extends React.Component< } public render() { - const { title, pullRequestNumber } = this.props.pullRequest + const { + dispatcher, + accounts, + repository, + pullRequest, + emoji, + review, + onSubmit, + onDismissed, + } = this.props - const header = ( -
- {this.renderPullRequestIcon()} - - {title}{' '} - #{pullRequestNumber}{' '} - -
- ) + const icon = getPullRequestReviewStateIcon(review.state) return ( - - -
- {this.renderTimelineItem()} - {this.renderCommentBubble()} -
- - {this.renderFooterContent()} - + ) } - private renderTimelineItem() { - const { review, repository } = this.props - const { user } = review - const { endpoint } = repository.gitHubRepository - const verb = getVerbForPullRequestReview(review) - const userAvatar = { - name: user.login, - email: getStealthEmailForUser(user.id, user.login, endpoint), - avatarURL: user.avatar_url, - endpoint: endpoint, - } - - const bottomLine = this.shouldRenderCommentBubble() - ? null - : this.renderDashedTimelineLine('bottom') - - const timelineItemClass = classNames('timeline-item', { - 'with-comment': this.shouldRenderCommentBubble(), - }) - - const submittedAt = new Date(review.submitted_at) - const diff = submittedAt.getTime() - Date.now() - const relativeReviewDate = formatRelative(diff) - - return ( -
- {this.renderDashedTimelineLine('top')} -
- - {this.renderReviewIcon()} -
- - {review.user.login} - {' '} - {verb} your pull request{' '} - - {relativeReviewDate} - -
-
- {bottomLine} -
- ) - } - - private shouldRenderCommentBubble() { - return this.props.review.body !== '' - } - - private renderCommentBubble() { - if (!this.shouldRenderCommentBubble()) { - return null - } - - return ( -
-
{this.renderReviewBody()}
- {this.renderDashedTimelineLine('bottom')} -
- ) - } - - private renderDashedTimelineLine(type: 'top' | 'bottom') { - return ( - - {/* Need to use 0.5 for X to prevent nearest neighbour filtering causing - the line to appear semi-transparent. */} - - - ) - } - - private renderFooterContent() { + private renderFooterContent = () => { const { review, shouldChangeRepository, shouldCheckoutBranch } = this.props const isApprovedReview = review.state === 'APPROVED' @@ -213,56 +132,6 @@ export class PullRequestReview extends React.Component< ) } - private onMarkdownLinkClicked = (url: string) => { - this.props.dispatcher.openInBrowser(url) - } - - private renderReviewBody() { - const { review, emoji, pullRequest } = this.props - const { base } = pullRequest - - return ( - - ) - } - - private renderPullRequestIcon = () => { - const { pullRequest } = this.props - - const cls = classNames('pull-request-icon', { - draft: pullRequest.draft, - }) - - return ( - - ) - } - - private renderReviewIcon = () => { - const { review } = this.props - - const icon = getPullRequestReviewStateIcon(review.state) - return ( -
- -
- ) - } - private onSubmit = async (event: React.MouseEvent) => { event.preventDefault() diff --git a/app/styles/_ui.scss b/app/styles/_ui.scss index f43653c03a..522b6b341c 100644 --- a/app/styles/_ui.scss +++ b/app/styles/_ui.scss @@ -95,7 +95,6 @@ @import 'ui/check-runs/_ci-check-run-popover'; @import 'ui/check-runs/ci-check-run-job-steps'; @import 'ui/_pull-request-checks-failed'; -@import 'ui/_pull-request-review'; @import 'ui/_sandboxed-markdown'; @import 'ui/_pull-request-quick-view'; @import 'ui/discard-changes-retry'; diff --git a/app/styles/ui/_dialog.scss b/app/styles/ui/_dialog.scss index 3669ea9fd1..0bb46b161f 100644 --- a/app/styles/ui/_dialog.scss +++ b/app/styles/ui/_dialog.scss @@ -22,6 +22,7 @@ @import 'dialogs/unreachable-commits'; @import 'dialogs/open-pull-request'; @import 'dialogs/installing-update'; +@import 'dialogs/pull-request-comment-like'; // The styles herein attempt to follow a flow where margins are only applied // to the bottom of elements (with the exception of the last child). This to diff --git a/app/styles/ui/_pull-request-review.scss b/app/styles/ui/dialogs/_pull-request-comment-like.scss similarity index 96% rename from app/styles/ui/_pull-request-review.scss rename to app/styles/ui/dialogs/_pull-request-comment-like.scss index 99cd84c151..e6247a138d 100644 --- a/app/styles/ui/_pull-request-review.scss +++ b/app/styles/ui/dialogs/_pull-request-comment-like.scss @@ -1,4 +1,5 @@ -#pull-request-review { +#pull-request-review, +#pull-request-comment { --avatar-size: 40px; min-width: 500px; @@ -6,7 +7,7 @@ height: unset; } - .pull-request-review-dialog-header { + .pull-request-comment-like-dialog-header { display: flex; flex-direction: row; align-items: center; @@ -44,7 +45,7 @@ max-height: 300px; overflow: auto; - .review-container { + .comment-container { .timeline-line { width: 1px; height: 24px; @@ -103,7 +104,7 @@ .link-button-component { color: unset; - &.reviewer { + &.author { font-weight: bold; } } From 786f814d5e29afd62dad720e39d35923f6faddf5 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Thu, 9 Feb 2023 13:20:49 +0100 Subject: [PATCH 18/23] Add new tool to easily test notifications from real data --- app/src/lib/api.ts | 67 ++ app/src/lib/stores/alive-store.ts | 17 +- .../lib/stores/notifications-debug-store.ts | 151 ++++ app/src/lib/stores/notifications-store.ts | 6 + app/src/models/popup.ts | 5 + app/src/ui/app.tsx | 32 +- app/src/ui/index.tsx | 8 + .../test-notifications/test-notifications.tsx | 666 ++++++++++++++++++ app/styles/ui/_dialog.scss | 1 + .../ui/dialogs/_test-notifications.scss | 75 ++ app/test/unit/app-test.tsx | 9 + 11 files changed, 1031 insertions(+), 6 deletions(-) create mode 100644 app/src/lib/stores/notifications-debug-store.ts create mode 100644 app/src/ui/test-notifications/test-notifications.tsx create mode 100644 app/styles/ui/dialogs/_test-notifications.scss diff --git a/app/src/lib/api.ts b/app/src/lib/api.ts index bc17c4bcd8..9898c5e974 100644 --- a/app/src/lib/api.ts +++ b/app/src/lib/api.ts @@ -508,6 +508,15 @@ export interface IAPIPullRequestReview { | 'CHANGES_REQUESTED' } +/** Represents both issue comments and PR review comments */ +export interface IAPIComment { + readonly id: number + readonly body: string + readonly html_url: string + readonly user: IAPIIdentity + readonly created_at: string +} + /** The metadata about a GitHub server. */ export interface IServerMetadata { /** @@ -1047,6 +1056,64 @@ export class API { } } + /** Fetches all reviews from a given pull request. */ + public async fetchPullRequestReviews( + owner: string, + name: string, + prNumber: string + ) { + try { + const path = `/repos/${owner}/${name}/pulls/${prNumber}/reviews` + const response = await this.request('GET', path) + return await parsedResponse(response) + } catch (e) { + log.debug( + `failed fetching PR reviews for ${owner}/${name}/pulls/${prNumber}`, + e + ) + return [] + } + } + + /** Fetches all review comments from a given pull request. */ + public async fetchPullRequestReviewComments( + owner: string, + name: string, + prNumber: string, + reviewId: string + ) { + try { + const path = `/repos/${owner}/${name}/pulls/${prNumber}/reviews/${reviewId}/comments` + const response = await this.request('GET', path) + return await parsedResponse(response) + } catch (e) { + log.debug( + `failed fetching PR review comments for ${owner}/${name}/pulls/${prNumber}`, + e + ) + return [] + } + } + + /** Fetches all comments from a given pull request. */ + public async fetchPullRequestComments( + owner: string, + name: string, + prNumber: string + ) { + try { + const path = `/repos/${owner}/${name}/pulls/${prNumber}/comments` + const response = await this.request('GET', path) + return await parsedResponse(response) + } catch (e) { + log.debug( + `failed fetching PR comments for ${owner}/${name}/pulls/${prNumber}`, + e + ) + return [] + } + } + /** * Get the combined status for the given ref. */ diff --git a/app/src/lib/stores/alive-store.ts b/app/src/lib/stores/alive-store.ts index 73b7a73dda..a77cd3eec5 100644 --- a/app/src/lib/stores/alive-store.ts +++ b/app/src/lib/stores/alive-store.ts @@ -30,10 +30,21 @@ export interface IDesktopPullRequestReviewSubmitAliveEvent { readonly review_id: string } +export interface IDesktopPullRequestCommentAliveEvent { + readonly type: 'pr-comment' + readonly subtype: 'review-comment' | 'issue-comment' + readonly timestamp: number + readonly owner: string + readonly repo: string + readonly pull_request_number: number + readonly comment_id: string +} + /** Represents an Alive event relevant to Desktop. */ export type DesktopAliveEvent = | IDesktopChecksFailedAliveEvent | IDesktopPullRequestReviewSubmitAliveEvent + | IDesktopPullRequestCommentAliveEvent interface IAliveSubscription { readonly account: Account readonly subscription: Subscription @@ -246,7 +257,11 @@ export class AliveStore { } const data = event.data as any as DesktopAliveEvent - if (data.type === 'pr-checks-failed' || data.type === 'pr-review-submit') { + if ( + data.type === 'pr-checks-failed' || + data.type === 'pr-review-submit' || + data.type === 'pr-comment' + ) { this.emitter.emit(this.ALIVE_EVENT_RECEIVED_EVENT, data) } } diff --git a/app/src/lib/stores/notifications-debug-store.ts b/app/src/lib/stores/notifications-debug-store.ts new file mode 100644 index 0000000000..65869cbf82 --- /dev/null +++ b/app/src/lib/stores/notifications-debug-store.ts @@ -0,0 +1,151 @@ +import { GitHubRepository } from '../../models/github-repository' +import { PullRequest } from '../../models/pull-request' +import { RepositoryWithGitHubRepository } from '../../models/repository' +import { API, IAPIComment } from '../api' +import { + isValidNotificationPullRequestReview, + ValidNotificationPullRequestReview, +} from '../valid-notification-pull-request-review' +import { AccountsStore } from './accounts-store' +import { NotificationsStore } from './notifications-store' +import { PullRequestCoordinator } from './pull-request-coordinator' + +/** + * This class allows the TestNotifications dialog to fetch real data to simulate + * notifications. + */ +export class NotificationsDebugStore { + public constructor( + private readonly accountsStore: AccountsStore, + private readonly notificationsStore: NotificationsStore, + private readonly pullRequestCoordinator: PullRequestCoordinator + ) {} + + private async getAccountForRepository(repository: GitHubRepository) { + const { endpoint } = repository + + const accounts = await this.accountsStore.getAll() + return accounts.find(a => a.endpoint === endpoint) ?? null + } + + private async getAPIForRepository(repository: GitHubRepository) { + const account = await this.getAccountForRepository(repository) + + if (account === null) { + return null + } + + return API.fromAccount(account) + } + + /** Fetch all pull requests for the given repository. */ + public async getPullRequests(repository: RepositoryWithGitHubRepository) { + return this.pullRequestCoordinator.getAllPullRequests(repository) + } + + /** Fetch all reviews for the given pull request. */ + public async getPullRequestReviews( + repository: RepositoryWithGitHubRepository, + pullRequestNumber: number + ) { + const api = await this.getAPIForRepository(repository.gitHubRepository) + if (api === null) { + return [] + } + + const ghRepository = repository.gitHubRepository + + const reviews = await api.fetchPullRequestReviews( + ghRepository.owner.login, + ghRepository.name, + pullRequestNumber.toString() + ) + + return reviews.filter(isValidNotificationPullRequestReview) + } + + /** Fetch all comments (issue and review comments) for the given pull request. */ + public async getPullRequestComments( + repository: RepositoryWithGitHubRepository, + pullRequestNumber: number + ) { + const api = await this.getAPIForRepository(repository.gitHubRepository) + if (api === null) { + return [] + } + + const ghRepository = repository.gitHubRepository + + const issueComments = await api.fetchPullRequestComments( + ghRepository.owner.login, + ghRepository.name, + pullRequestNumber.toString() + ) + + const issueCommentIds = new Set(issueComments.map(c => c.id)) + + // Fetch review comments of type COMMENTED and with no body + const allReviews = await api.fetchPullRequestReviews( + ghRepository.owner.login, + ghRepository.name, + pullRequestNumber.toString() + ) + + const commentedReviewsWithNoBody = allReviews.filter( + review => review.state === 'COMMENTED' && !review.body + ) + + const allReviewComments = await Promise.all( + commentedReviewsWithNoBody.map(review => + api.fetchPullRequestReviewComments( + ghRepository.owner.login, + ghRepository.name, + pullRequestNumber.toString(), + review.id.toString() + ) + ) + ) + + // Only reviews with one comment, and that comment is not an issue comment + const singleReviewComments = allReviewComments + .flatMap(comments => (comments.length === 1 ? comments : [])) + .filter(comment => !issueCommentIds.has(comment.id)) + + return [...issueComments, ...singleReviewComments] + } + + /** Simulate a notification for the given pull request review. */ + public simulatePullRequestReviewNotification( + repository: GitHubRepository, + pullRequest: PullRequest, + review: ValidNotificationPullRequestReview + ) { + this.notificationsStore.simulateAliveEvent({ + type: 'pr-review-submit', + timestamp: new Date(review.submitted_at).getTime(), + owner: repository.owner.login, + repo: repository.name, + pull_request_number: pullRequest.pullRequestNumber, + state: review.state, + review_id: review.id.toString(), + }) + } + + /** Simulate a notification for the given pull request comment. */ + public simulatePullRequestCommentNotification( + repository: GitHubRepository, + pullRequest: PullRequest, + comment: IAPIComment, + isIssueComment: boolean + ) { + this.notificationsStore.simulateAliveEvent({ + type: 'pr-comment', + subtype: isIssueComment ? 'issue-comment' : 'review-comment', + timestamp: new Date(comment.created_at).getTime(), + owner: repository.owner.login, + repo: repository.name, + pull_request_number: pullRequest.pullRequestNumber, + comment_id: comment.id.toString(), + }) + } +} diff --git a/app/src/lib/stores/notifications-store.ts b/app/src/lib/stores/notifications-store.ts index fc3a37f88e..93bb4e2742 100644 --- a/app/src/lib/stores/notifications-store.ts +++ b/app/src/lib/stores/notifications-store.ts @@ -104,6 +104,12 @@ export class NotificationsStore { public onNotificationEventReceived: NotificationCallback = async (event, id, userInfo) => this.handleAliveEvent(userInfo, true) + public simulateAliveEvent(event: DesktopAliveEvent) { + if (__DEV__) { + this.handleAliveEvent(event, false) + } + } + private async handleAliveEvent( e: DesktopAliveEvent, skipNotification: boolean diff --git a/app/src/models/popup.ts b/app/src/models/popup.ts index 96d9a16f2a..abdb78ff7e 100644 --- a/app/src/models/popup.ts +++ b/app/src/models/popup.ts @@ -89,6 +89,7 @@ export enum PopupType { StartPullRequest = 'StartPullRequest', Error = 'Error', InstallingUpdate = 'InstallingUpdate', + TestNotifications = 'TestNotifications', } interface IBasePopup { @@ -388,5 +389,9 @@ export type PopupDetail = | { type: PopupType.InstallingUpdate } + | { + type: PopupType.TestNotifications + repository: RepositoryWithGitHubRepository + } export type Popup = IBasePopup & PopupDetail diff --git a/app/src/ui/app.tsx b/app/src/ui/app.tsx index 0fb3f95fd0..4ec2feda18 100644 --- a/app/src/ui/app.tsx +++ b/app/src/ui/app.tsx @@ -148,7 +148,6 @@ import { WarnForcePushDialog } from './multi-commit-operation/dialog/warn-force- import { clamp } from '../lib/clamp' import { generateRepositoryListContextMenu } from './repositories-list/repository-list-item-context-menu' import * as ipcRenderer from '../lib/ipc-renderer' -import { showNotification } from '../lib/notifications/show-notification' import { DiscardChangesRetryDialog } from './discard-changes/discard-changes-retry-dialog' import { generateDevReleaseSummary } from '../lib/release-notes' import { PullRequestReview } from './notifications/pull-request-review' @@ -164,6 +163,8 @@ import { uuid } from '../lib/uuid' import { InstallingUpdate } from './installing-update/installing-update' import { enableStackedPopups } from '../lib/feature-flag' import { DialogStackContext } from './dialog' +import { TestNotifications } from './test-notifications/test-notifications' +import { NotificationsDebugStore } from '../lib/stores/notifications-debug-store' const MinuteInMilliseconds = 1000 * 60 const HourInMilliseconds = MinuteInMilliseconds * 60 @@ -185,6 +186,7 @@ interface IAppProps { readonly issuesStore: IssuesStore readonly gitHubUserStore: GitHubUserStore readonly aheadBehindStore: AheadBehindStore + readonly notificationsDebugStore: NotificationsDebugStore readonly startTime: number } @@ -489,10 +491,19 @@ export class App extends React.Component { return } - showNotification({ - title: 'Test notification', - body: 'Click here! This is a test notification', - onClick: () => this.props.dispatcher.showPopup({ type: PopupType.About }), + // if current repository is not repository with github repository, return + const repository = this.getRepository() + if ( + repository == null || + repository instanceof CloningRepository || + !isRepositoryWithGitHubRepository(repository) + ) { + return + } + + this.props.dispatcher.showPopup({ + type: PopupType.TestNotifications, + repository, }) } @@ -2373,6 +2384,17 @@ export class App extends React.Component { /> ) } + case PopupType.TestNotifications: { + return ( + + ) + } default: return assertNever(popup, `Unknown popup type: ${popup}`) } diff --git a/app/src/ui/index.tsx b/app/src/ui/index.tsx index caa09e662f..7e8a7f1234 100644 --- a/app/src/ui/index.tsx +++ b/app/src/ui/index.tsx @@ -79,6 +79,7 @@ import { NotificationsStore } from '../lib/stores/notifications-store' import * as ipcRenderer from '../lib/ipc-renderer' import { migrateRendererGUID } from '../lib/get-renderer-guid' import { initializeRendererNotificationHandler } from '../lib/notifications/notification-handler' +import { NotificationsDebugStore } from '../lib/stores/notifications-debug-store' if (__DEV__) { installDevGlobals() @@ -267,6 +268,12 @@ const notificationsStore = new NotificationsStore( statsStore ) +const notificationsDebugStore = new NotificationsDebugStore( + accountsStore, + notificationsStore, + pullRequestCoordinator +) + const appStore = new AppStore( gitHubUserStore, cloningRepositoriesStore, @@ -353,6 +360,7 @@ ReactDOM.render( issuesStore={issuesStore} gitHubUserStore={gitHubUserStore} aheadBehindStore={aheadBehindStore} + notificationsDebugStore={notificationsDebugStore} startTime={startTime} />, document.getElementById('desktop-app-container')! diff --git a/app/src/ui/test-notifications/test-notifications.tsx b/app/src/ui/test-notifications/test-notifications.tsx new file mode 100644 index 0000000000..070a16c3a8 --- /dev/null +++ b/app/src/ui/test-notifications/test-notifications.tsx @@ -0,0 +1,666 @@ +import classNames from 'classnames' +import React from 'react' +import { getHTMLURL, IAPIComment } from '../../lib/api' +import { assertNever } from '../../lib/fatal-error' +import { NotificationsDebugStore } from '../../lib/stores/notifications-debug-store' +import { + ValidNotificationPullRequestReview, + ValidNotificationPullRequestReviewState, +} from '../../lib/valid-notification-pull-request-review' +import { PullRequest } from '../../models/pull-request' +import { RepositoryWithGitHubRepository } from '../../models/repository' +import { + Dialog, + DialogContent, + DialogFooter, + OkCancelButtonGroup, +} from '../dialog' +import { DialogHeader } from '../dialog/header' +import { Dispatcher } from '../dispatcher' +import { Button } from '../lib/button' +import { List } from '../lib/list' +import { Loading } from '../lib/loading' +import { getPullRequestReviewStateIcon } from '../notifications/pull-request-review-helpers' +import { Octicon } from '../octicons' +import * as OcticonSymbol from '../octicons/octicons.generated' + +enum TestNotificationType { + PullRequestReview, + PullRequestReviewComment, +} + +enum TestNotificationStepKind { + SelectPullRequest, + SelectPullRequestReview, + SelectPullRequestComment, +} + +type TestNotificationFlow = { + readonly type: TestNotificationType + readonly steps: ReadonlyArray +} + +const TestNotificationFlows: ReadonlyArray = [ + { + type: TestNotificationType.PullRequestReview, + steps: [ + TestNotificationStepKind.SelectPullRequest, + TestNotificationStepKind.SelectPullRequestReview, + ], + }, + { + type: TestNotificationType.PullRequestReviewComment, + steps: [ + TestNotificationStepKind.SelectPullRequest, + TestNotificationStepKind.SelectPullRequestComment, + ], + }, +] + +type TestNotificationStepSelectPullRequestResult = { + readonly kind: TestNotificationStepKind.SelectPullRequest + readonly pullRequest: PullRequest +} +type TestNotificationStepSelectPullRequestReviewResult = { + readonly kind: TestNotificationStepKind.SelectPullRequestReview + readonly review: ValidNotificationPullRequestReview +} +type TestNotificationStepSelectPullRequestCommentResult = { + readonly kind: TestNotificationStepKind.SelectPullRequestComment + readonly comment: IAPIComment + readonly isIssueComment: boolean +} + +type TestNotificationStepResultMap = Map< + TestNotificationStepKind.SelectPullRequest, + TestNotificationStepSelectPullRequestResult +> & + Map< + TestNotificationStepKind.SelectPullRequestReview, + TestNotificationStepSelectPullRequestReviewResult + > & + Map< + TestNotificationStepKind.SelectPullRequestComment, + TestNotificationStepSelectPullRequestCommentResult + > + +interface ITestNotificationsState { + readonly selectedFlow: TestNotificationFlow | null + readonly stepResults: TestNotificationStepResultMap + readonly loading: boolean + readonly pullRequests: ReadonlyArray + readonly reviews: ReadonlyArray + readonly comments: ReadonlyArray +} + +interface ITestNotificationsProps { + readonly dispatcher: Dispatcher + readonly notificationsDebugStore: NotificationsDebugStore + readonly repository: RepositoryWithGitHubRepository + readonly onDismissed: () => void +} + +class TestNotificationItemRowContent extends React.Component<{ + readonly leftAccessory?: JSX.Element + readonly html_url?: string + readonly dispatcher: Dispatcher +}> { + public render() { + const { leftAccessory, html_url, children } = this.props + + return ( +
+ {leftAccessory &&
{leftAccessory}
} +
{children}
+ {html_url && ( +
+ +
+ )} +
+ ) + } + + private onExternalLinkClick = (e: React.MouseEvent) => { + const { dispatcher, html_url } = this.props + + if (html_url === undefined) { + return + } + + e.stopPropagation() + dispatcher.openInBrowser(html_url) + } +} + +export class TestNotifications extends React.Component< + ITestNotificationsProps, + ITestNotificationsState +> { + public constructor(props: ITestNotificationsProps) { + super(props) + + this.state = { + selectedFlow: null, + stepResults: new Map(), + loading: false, + pullRequests: [], + reviews: [], + comments: [], + } + } + + private onDismissed = () => { + this.props.dispatcher.closePopup() + } + + private renderNotificationType = ( + type: TestNotificationType + ): JSX.Element => { + const title = + type === TestNotificationType.PullRequestReview + ? 'Pull Request Review' + : 'Pull Request Review Comment' + + return ( + + ) + } + + private getOnNotificationTypeClick = (type: TestNotificationType) => () => { + const selectedFlow = + TestNotificationFlows.find(f => f.type === type) ?? null + + this.setState( + { + selectedFlow, + }, + () => { + this.prepareForNextStep() + } + ) + } + + private doFinalAction() { + const selectedFlow = this.state.selectedFlow + + if (selectedFlow === null) { + return + } + + switch (selectedFlow.type) { + case TestNotificationType.PullRequestReview: { + const pullRequestNumber = this.getPullRequest() + const review = this.getReview() + + if (pullRequestNumber === null || review === null) { + return + } + + this.props.notificationsDebugStore.simulatePullRequestReviewNotification( + this.props.repository.gitHubRepository, + pullRequestNumber, + review + ) + break + } + case TestNotificationType.PullRequestReviewComment: { + const pullRequest = this.getPullRequest() + const commentInfo = this.getCommentInfo() + + if (pullRequest === null || commentInfo === null) { + return + } + + const { comment, isIssueComment } = commentInfo + + this.props.notificationsDebugStore.simulatePullRequestCommentNotification( + this.props.repository.gitHubRepository, + pullRequest, + comment, + isIssueComment + ) + break + } + default: + assertNever(selectedFlow.type, `Unknown flow type: ${selectedFlow}`) + } + } + + private prepareForNextStep() { + const nextStep = this.state.selectedFlow?.steps[this.state.stepResults.size] + + if (nextStep === undefined) { + this.doFinalAction() + this.back() + return + } + + switch (nextStep) { + case TestNotificationStepKind.SelectPullRequest: { + this.setState({ + loading: true, + }) + + this.props.notificationsDebugStore + .getPullRequests(this.props.repository) + .then(pullRequests => { + this.setState({ + pullRequests, + loading: false, + }) + }) + break + } + case TestNotificationStepKind.SelectPullRequestReview: { + this.setState({ + loading: true, + }) + + const pullRequest = this.getPullRequest() + + if (pullRequest === null) { + return + } + + this.props.notificationsDebugStore + .getPullRequestReviews( + this.props.repository, + pullRequest.pullRequestNumber + ) + .then(reviews => { + this.setState({ + reviews, + loading: false, + }) + }) + break + } + case TestNotificationStepKind.SelectPullRequestComment: { + this.setState({ + loading: true, + }) + + const pullRequest = this.getPullRequest() + + if (pullRequest === null) { + return + } + + this.props.notificationsDebugStore + .getPullRequestComments( + this.props.repository, + pullRequest.pullRequestNumber + ) + .then(comments => { + this.setState({ + comments, + loading: false, + }) + }) + break + } + default: + assertNever(nextStep, `Unknown step: ${nextStep}`) + } + } + + private getPullRequest(): PullRequest | null { + const pullRequestResult = this.state.stepResults.get( + TestNotificationStepKind.SelectPullRequest + ) + + if (pullRequestResult === undefined) { + return null + } + + return pullRequestResult.pullRequest + } + + private getReview(): ValidNotificationPullRequestReview | null { + const reviewResult = this.state.stepResults.get( + TestNotificationStepKind.SelectPullRequestReview + ) + + if (reviewResult === undefined) { + return null + } + + return reviewResult.review + } + + private getCommentInfo() { + const commentResult = this.state.stepResults.get( + TestNotificationStepKind.SelectPullRequestComment + ) + + if (commentResult === undefined) { + return null + } + + return { + comment: commentResult.comment, + isIssueComment: commentResult.isIssueComment, + } + } + + private renderCurrentStep() { + if (this.state.selectedFlow === null) { + return ( +
+

Select the type of notification to display:

+
+ {this.renderNotificationType( + TestNotificationType.PullRequestReview + )} + {this.renderNotificationType( + TestNotificationType.PullRequestReviewComment + )} +
+
+ ) + } + + const currentStep = this.state.selectedFlow.steps.at( + this.state.stepResults.size + ) + + if (currentStep === undefined) { + return

Done!

+ } + + switch (currentStep) { + case TestNotificationStepKind.SelectPullRequest: + return this.renderSelectPullRequest() + case TestNotificationStepKind.SelectPullRequestReview: + return this.renderSelectPullRequestReview() + case TestNotificationStepKind.SelectPullRequestComment: + return this.renderSelectPullRequestComment() + default: + return assertNever(currentStep, `Unknown step: ${currentStep}`) + } + } + + private renderSelectPullRequest() { + if (this.state.loading) { + return + } + + const { pullRequests } = this.state + + if (pullRequests.length === 0) { + return

No pull requests found

+ } + + return ( +
+ Pull requests: + +
+ ) + } + + private onPullRequestRowClick = (row: number) => { + const pullRequest = this.state.pullRequests[row] + const stepResults = this.state.stepResults + stepResults.set(TestNotificationStepKind.SelectPullRequest, { + kind: TestNotificationStepKind.SelectPullRequest, + pullRequest: pullRequest, + }) + + this.setState( + { + stepResults, + }, + () => { + this.prepareForNextStep() + } + ) + } + + private renderSelectPullRequestReview() { + if (this.state.loading) { + return + } + + const { reviews } = this.state + + if (reviews.length === 0) { + return

No reviews found

+ } + + return ( +
+ Reviews: + +
+ ) + } + + private onPullRequestReviewRowClick = (row: number) => { + const review = this.state.reviews[row] + const stepResults = this.state.stepResults + stepResults.set(TestNotificationStepKind.SelectPullRequestReview, { + kind: TestNotificationStepKind.SelectPullRequestReview, + review: review, + }) + + this.setState( + { + stepResults, + }, + () => { + this.prepareForNextStep() + } + ) + } + + private renderSelectPullRequestComment() { + if (this.state.loading) { + return + } + + const { comments } = this.state + + if (comments.length === 0) { + return

No comments found

+ } + + return ( +
+ Comments: + +
+ ) + } + + private onPullRequestCommentRowClick = (row: number) => { + const comment = this.state.comments[row] + const stepResults = this.state.stepResults + stepResults.set(TestNotificationStepKind.SelectPullRequestComment, { + kind: TestNotificationStepKind.SelectPullRequestComment, + comment: comment, + isIssueComment: comment.html_url.includes('#issuecomment-'), + }) + + this.setState( + { + stepResults, + }, + () => { + this.prepareForNextStep() + } + ) + } + + private renderPullRequestCommentRow = (row: number) => { + const comment = this.state.comments[row] + return ( + + {comment.body} +
+ by {comment.user.login} + + ) + } + + private renderPullRequestReviewRow = (row: number) => { + const review = this.state.reviews[row] + + return ( + + {review.body || Review without body} +
+ by {review.user.login} + + ) + } + + private renderReviewStateIcon = ( + state: ValidNotificationPullRequestReviewState + ) => { + const icon = getPullRequestReviewStateIcon(state) + return ( +
+ +
+ ) + } + + private renderPullRequestRow = (row: number) => { + const pullRequest = this.state.pullRequests[row] + const repository = this.props.repository.gitHubRepository + const endpointHtmlUrl = getHTMLURL(repository.endpoint) + const htmlURL = `${endpointHtmlUrl}/${repository.owner.login}/${repository.name}/pull/${pullRequest.pullRequestNumber}` + + return ( + + + #{pullRequest.pullRequestNumber} + {pullRequest.draft ? ' (Draft)' : ''}: + {' '} + {pullRequest.title}
+ by {pullRequest.author} + + ) + } + + private renderPullRequestStateIcon = ( + pullRequest: PullRequest + ): JSX.Element => { + return ( + + ) + } + + public render() { + return ( + + + {this.renderCurrentStep()} + + + + + ) + } + + private onBack = (event: React.MouseEvent) => { + this.back() + event.preventDefault() + } + + private back() { + const { selectedFlow, stepResults } = this.state + if (selectedFlow === null) { + return + } + + if (stepResults.size === 0) { + this.setState( + { + selectedFlow: null, + stepResults: new Map(), + }, + () => { + this.prepareForNextStep() + } + ) + } + + const steps = selectedFlow.steps + const lastStep = steps.at(stepResults.size - 1) + if (lastStep === undefined) { + return + } + + const newStepResults: Map = new Map( + stepResults + ) + newStepResults.delete(lastStep) + + this.setState( + { + stepResults: newStepResults as TestNotificationStepResultMap, + }, + () => { + this.prepareForNextStep() + } + ) + } +} diff --git a/app/styles/ui/_dialog.scss b/app/styles/ui/_dialog.scss index 0bb46b161f..8613a0f617 100644 --- a/app/styles/ui/_dialog.scss +++ b/app/styles/ui/_dialog.scss @@ -22,6 +22,7 @@ @import 'dialogs/unreachable-commits'; @import 'dialogs/open-pull-request'; @import 'dialogs/installing-update'; +@import 'dialogs/test-notifications'; @import 'dialogs/pull-request-comment-like'; // The styles herein attempt to follow a flow where margins are only applied diff --git a/app/styles/ui/dialogs/_test-notifications.scss b/app/styles/ui/dialogs/_test-notifications.scss new file mode 100644 index 0000000000..00cb1cfa01 --- /dev/null +++ b/app/styles/ui/dialogs/_test-notifications.scss @@ -0,0 +1,75 @@ +#test-notifications { + .notification-type-list { + display: flex; + flex-direction: column; + row-gap: var(--spacing-half); + } + + .list { + height: 200px; + + .row-content { + width: 100%; + display: flex; + flex-direction: row; + + .left-accessory { + flex-grow: 0; + flex-shrink: 0; + align-self: center; + margin-right: 10px; + } + + .main-content { + flex-grow: 1; + white-space: nowrap; + text-overflow: ellipsis; + overflow: hidden; + } + + .right-accessory { + flex-grow: 0; + flex-shrink: 0; + align-self: center; + margin-left: 10px; + } + } + } + + .review-icon-container { + border-radius: 50%; + width: 30px; + height: 30px; + margin: 0; + display: flex; + justify-content: center; + align-items: center; + + .octicon { + width: 16px; + height: 16px; + text-align: center; + } + + &.pr-review-approved { + color: var(--pr-approved-icon-color); + background-color: var(--pr-approved-icon-background-color); + } + &.pr-review-changes-requested { + color: var(--pr-changes-requested-icon-color); + background-color: var(--pr-changes-requested-icon-background-color); + } + &.pr-review-commented { + color: var(--pr-commented-icon-color); + background-color: var(--pr-commented-icon-background-color); + } + } + + .pr-icon { + color: var(--pr-open-icon-color); + } + + .pr-draft-icon { + color: var(--pr-draft-icon-color); + } +} diff --git a/app/test/unit/app-test.tsx b/app/test/unit/app-test.tsx index 095dbd2ca1..3a2610d9f5 100644 --- a/app/test/unit/app-test.tsx +++ b/app/test/unit/app-test.tsx @@ -32,6 +32,7 @@ import { CommitStatusStore } from '../../src/lib/stores/commit-status-store' import { AheadBehindStore } from '../../src/lib/stores/ahead-behind-store' import { AliveStore } from '../../src/lib/stores/alive-store' import { NotificationsStore } from '../../src/lib/stores/notifications-store' +import { NotificationsDebugStore } from '../../src/lib/stores/notifications-debug-store' describe('App', () => { let appStore: AppStore @@ -41,6 +42,7 @@ describe('App', () => { let githubUserStore: GitHubUserStore let issuesStore: IssuesStore let aheadBehindStore: AheadBehindStore + let notificationsDebugStore: NotificationsDebugStore beforeEach(async () => { const db = new TestGitHubUserDatabase() @@ -86,6 +88,12 @@ describe('App', () => { ) notificationsStore.setNotificationsEnabled(false) + notificationsDebugStore = new NotificationsDebugStore( + accountsStore, + notificationsStore, + pullRequestCoordinator + ) + appStore = new AppStore( githubUserStore, new CloningRepositoriesStore(), @@ -117,6 +125,7 @@ describe('App', () => { issuesStore={issuesStore} gitHubUserStore={githubUserStore} aheadBehindStore={aheadBehindStore} + notificationsDebugStore={notificationsDebugStore} startTime={0} /> ) as unknown as React.Component From 05f3631a71b0ffff265d53322282f56a6396678c Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Thu, 9 Feb 2023 09:21:19 -0500 Subject: [PATCH 19/23] Make commit message avatar into a button --- app/src/ui/changes/commit-message-avatar.tsx | 8 +++----- app/styles/ui/_commit-message-avatar.scss | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/app/src/ui/changes/commit-message-avatar.tsx b/app/src/ui/changes/commit-message-avatar.tsx index a015c2e394..043c37ffea 100644 --- a/app/src/ui/changes/commit-message-avatar.tsx +++ b/app/src/ui/changes/commit-message-avatar.tsx @@ -1,5 +1,3 @@ -/* eslint-disable jsx-a11y/no-static-element-interactions */ -/* eslint-disable jsx-a11y/click-events-have-key-events */ import React from 'react' import { Select } from '../lib/select' import { Button } from '../lib/button' @@ -73,10 +71,10 @@ export class CommitMessageAvatar extends React.Component< public render() { return (
-
+
+ {this.state.isPopoverOpen && this.renderPopover()}
) @@ -108,7 +106,7 @@ export class CommitMessageAvatar extends React.Component< }) } - private onAvatarClick = (event: React.FormEvent) => { + private onAvatarClick = (event: React.FormEvent) => { if (this.props.warningBadgeVisible === false) { return } diff --git a/app/styles/ui/_commit-message-avatar.scss b/app/styles/ui/_commit-message-avatar.scss index 1302bdf96c..1e937a31a1 100644 --- a/app/styles/ui/_commit-message-avatar.scss +++ b/app/styles/ui/_commit-message-avatar.scss @@ -2,6 +2,21 @@ // With this, the popover's absolute position will be relative to its parent position: relative; + // override default button styles + .button-component { + overflow: inherit; + text-overflow: inherit; + white-space: inherit; + font-family: inherit; + font-size: inherit; + padding: inherit; + border: none; + height: inherit; + color: inherit; + background-color: none; + border-radius: none; + } + .warning-badge { background-color: var(--commit-warning-badge-background-color); border: var(--commit-warning-badge-border-color) 1px solid; From 2ef16d031de609b28c3937391f07562a9a01d16e Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Thu, 9 Feb 2023 09:27:45 -0500 Subject: [PATCH 20/23] Add better, targeted styles --- app/src/ui/changes/commit-message-avatar.tsx | 2 +- app/styles/ui/_commit-message-avatar.scss | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/src/ui/changes/commit-message-avatar.tsx b/app/src/ui/changes/commit-message-avatar.tsx index 043c37ffea..93d39fe4cd 100644 --- a/app/src/ui/changes/commit-message-avatar.tsx +++ b/app/src/ui/changes/commit-message-avatar.tsx @@ -71,7 +71,7 @@ export class CommitMessageAvatar extends React.Component< public render() { return (
- diff --git a/app/styles/ui/_commit-message-avatar.scss b/app/styles/ui/_commit-message-avatar.scss index 1e937a31a1..be7508e63f 100644 --- a/app/styles/ui/_commit-message-avatar.scss +++ b/app/styles/ui/_commit-message-avatar.scss @@ -3,7 +3,7 @@ position: relative; // override default button styles - .button-component { + .avatar-button { overflow: inherit; text-overflow: inherit; white-space: inherit; @@ -15,6 +15,11 @@ color: inherit; background-color: none; border-radius: none; + margin-right: var(--spacing-half); + + .avatar { + margin-right: 0; + } } .warning-badge { From a441ac72f2ddc3da2246280c32d68b536a97a644 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Thu, 9 Feb 2023 12:22:01 -0500 Subject: [PATCH 21/23] Make border-radius of avatar button match avatar image --- app/styles/ui/_commit-message-avatar.scss | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/styles/ui/_commit-message-avatar.scss b/app/styles/ui/_commit-message-avatar.scss index be7508e63f..bb32cd47f5 100644 --- a/app/styles/ui/_commit-message-avatar.scss +++ b/app/styles/ui/_commit-message-avatar.scss @@ -2,8 +2,8 @@ // With this, the popover's absolute position will be relative to its parent position: relative; - // override default button styles .avatar-button { + // override default button styles overflow: inherit; text-overflow: inherit; white-space: inherit; @@ -14,7 +14,8 @@ height: inherit; color: inherit; background-color: none; - border-radius: none; + + border-radius: 50%; margin-right: var(--spacing-half); .avatar { From a72cb4907c4ca0ac1ae595d1818aefb801cb6723 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Fri, 10 Feb 2023 12:02:56 -0500 Subject: [PATCH 22/23] Avatar only a button when it needs to be --- app/src/ui/changes/commit-message-avatar.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/app/src/ui/changes/commit-message-avatar.tsx b/app/src/ui/changes/commit-message-avatar.tsx index 93d39fe4cd..0b4830f858 100644 --- a/app/src/ui/changes/commit-message-avatar.tsx +++ b/app/src/ui/changes/commit-message-avatar.tsx @@ -71,10 +71,17 @@ export class CommitMessageAvatar extends React.Component< public render() { return (
- + )} + + {!this.props.warningBadgeVisible && ( - + )} + {this.state.isPopoverOpen && this.renderPopover()}
) From 5c7f4a788f77b222d1de90b0d48bba403ffc1f97 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Mon, 13 Feb 2023 11:48:21 +0100 Subject: [PATCH 23/23] Focus on first suitable child in sign in flow When switching steps in the sign in flow we're essentially showing a new dialog (or at least it should behave as such) which means we'll need to reset the focus, if we don't then keyboard focus will just flow to the body whenever we change the dialog contents. --- app/src/ui/dialog/dialog.tsx | 2 +- app/src/ui/sign-in/sign-in.tsx | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/app/src/ui/dialog/dialog.tsx b/app/src/ui/dialog/dialog.tsx index ff7b39598d..94840c9802 100644 --- a/app/src/ui/dialog/dialog.tsx +++ b/app/src/ui/dialog/dialog.tsx @@ -383,7 +383,7 @@ export class Dialog extends React.Component { * 4. Any remaining button * */ - private focusFirstSuitableChild() { + public focusFirstSuitableChild() { const dialog = this.dialogElement if (dialog === null) { diff --git a/app/src/ui/sign-in/sign-in.tsx b/app/src/ui/sign-in/sign-in.tsx index 9ffe0e6321..0f9689dfc5 100644 --- a/app/src/ui/sign-in/sign-in.tsx +++ b/app/src/ui/sign-in/sign-in.tsx @@ -41,6 +41,8 @@ const SignInWithBrowserTitle = __DARWIN__ const DefaultTitle = 'Sign in' export class SignIn extends React.Component { + private readonly dialogRef = React.createRef() + public constructor(props: ISignInProps) { super(props) @@ -52,6 +54,18 @@ export class SignIn extends React.Component { } } + public componentDidUpdate(prevProps: ISignInProps) { + // Whenever the sign in step changes we replace the dialog contents which + // means we need to re-focus the first suitable child element as it's + // essentially a "new" dialog we're showing only the dialog component itself + // doesn't know that. + if (prevProps.signInState !== null && this.props.signInState !== null) { + if (prevProps.signInState.kind !== this.props.signInState.kind) { + this.dialogRef.current?.focusFirstSuitableChild() + } + } + } + public componentWillReceiveProps(nextProps: ISignInProps) { if (nextProps.signInState !== this.props.signInState) { if ( @@ -161,6 +175,7 @@ export class SignIn extends React.Component { ) @@ -324,6 +339,7 @@ export class SignIn extends React.Component { onDismissed={this.onDismissed} onSubmit={this.onSubmit} loading={state.loading} + ref={this.dialogRef} > {errors} {this.renderStep()}