diff --git a/.eslintrc.yml b/.eslintrc.yml index 03051380c5..fe2ca1172b 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -15,6 +15,7 @@ extends: - prettier/react - plugin:@typescript-eslint/recommended - prettier/@typescript-eslint + - plugin:github/react rules: ########## @@ -29,6 +30,7 @@ rules: ########### # PLUGINS # ########### + # TYPESCRIPT '@typescript-eslint/naming-convention': - error diff --git a/.markdownlint.js b/.markdownlint.js new file mode 100644 index 0000000000..eb043f42c4 --- /dev/null +++ b/.markdownlint.js @@ -0,0 +1,2 @@ +const markdownlintGitHub = require('@github/markdownlint-github') +module.exports = markdownlintGitHub.init() diff --git a/app/package.json b/app/package.json index 9ca07a71f6..6456cc0bfd 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.0.6", + "version": "3.0.7", "main": "./main.js", "repository": { "type": "git", diff --git a/app/src/crash/crash-app.tsx b/app/src/crash/crash-app.tsx index a52636f228..1d2c861a92 100644 --- a/app/src/crash/crash-app.tsx +++ b/app/src/crash/crash-app.tsx @@ -201,7 +201,9 @@ export class CrashApp extends React.Component { } private renderBackgroundGraphics() { - return + return ( + + ) } public render() { diff --git a/app/src/lib/editors/darwin.ts b/app/src/lib/editors/darwin.ts index 363dbefccb..a13813951b 100644 --- a/app/src/lib/editors/darwin.ts +++ b/app/src/lib/editors/darwin.ts @@ -31,6 +31,10 @@ const editors: IDarwinExternalEditor[] = [ name: 'MacVim', bundleIdentifiers: ['org.vim.MacVim'], }, + { + name: 'Neovide', + bundleIdentifiers: ['com.neovide.neovide'], + }, { name: 'Visual Studio Code', bundleIdentifiers: ['com.microsoft.VSCode'], diff --git a/app/src/lib/feature-flag.ts b/app/src/lib/feature-flag.ts index cdbdec9cd2..24a112a0a2 100644 --- a/app/src/lib/feature-flag.ts +++ b/app/src/lib/feature-flag.ts @@ -68,16 +68,6 @@ export function enableUpdateFromEmulatedX64ToARM64(): boolean { return enableBetaFeatures() } -/** - * Should we allow x64 apps running under ARM translation to auto-update to - * ARM64 builds IMMEDIATELY instead of waiting for the next release? - */ -export function enableImmediateUpdateFromEmulatedX64ToARM64(): boolean { - // Because of how Squirrel.Windows works, this is only available for macOS. - // See: https://github.com/desktop/desktop/pull/14998 - return __DARWIN__ && enableBetaFeatures() -} - /** Should we allow resetting to a previous commit? */ export function enableResetToCommit(): boolean { return enableDevelopmentFeatures() @@ -112,3 +102,8 @@ export function enablePullRequestQuickView(): boolean { export function enableMultiCommitDiffs(): boolean { return enableBetaFeatures() } + +/** Should we enable the new interstitial for submodule diffs? */ +export function enableSubmoduleDiff(): boolean { + return enableBetaFeatures() +} diff --git a/app/src/lib/git/apply.ts b/app/src/lib/git/apply.ts index 8546731282..bf3680cce1 100644 --- a/app/src/lib/git/apply.ts +++ b/app/src/lib/git/apply.ts @@ -64,6 +64,7 @@ export async function applyPatchToIndex( const { kind } = diff switch (diff.kind) { case DiffType.Binary: + case DiffType.Submodule: case DiffType.Image: throw new Error( `Can't create partial commit in binary file: ${file.path}` diff --git a/app/src/lib/git/diff.ts b/app/src/lib/git/diff.ts index fe08083722..80dc153e53 100644 --- a/app/src/lib/git/diff.ts +++ b/app/src/lib/git/diff.ts @@ -8,6 +8,7 @@ import { FileChange, AppFileStatusKind, CommittedFileChange, + SubmoduleStatus, } from '../../models/status' import { DiffType, @@ -18,7 +19,6 @@ import { LineEndingsChange, parseLineEndingText, ILargeTextDiff, - IUnrenderableDiff, } from '../../models/diff' import { spawnAndComplete } from './spawn' @@ -32,6 +32,7 @@ import { git } from './core' import { NullTreeSHA } from './diff-index' import { GitError } from 'dugite' import { parseRawLogWithNumstat } from './log' +import { getConfigValue } from './config' /** * V8 has a limit on the size of string it can create (~256MB), and unless we want to @@ -480,16 +481,68 @@ function diffFromRawDiffOutput(output: Buffer): IRawDiff { return parser.parse(forceUnwrap(`Invalid diff output`, pieces.at(-1))) } -function buildDiff( +async function buildSubmoduleDiff( + buffer: Buffer, + repository: Repository, + file: FileChange, + status: SubmoduleStatus +): Promise { + const path = file.path + const fullPath = Path.join(repository.path, path) + const url = + (await getConfigValue(repository, `submodule.${path}.url`, true)) ?? '' + + let oldSHA = null + let newSHA = null + + if (status.commitChanged) { + const diff = buffer.toString('utf-8') + const lines = diff.split('\n') + const baseRegex = 'Subproject commit ([^-]+)(-dirty)?$' + const oldSHARegex = new RegExp('-' + baseRegex) + const newSHARegex = new RegExp('\\+' + baseRegex) + const lineMatch = (regex: RegExp) => + lines + .flatMap(line => { + const match = line.match(regex) + return match ? match[1] : [] + }) + .at(0) ?? null + + oldSHA = lineMatch(oldSHARegex) + newSHA = lineMatch(newSHARegex) + } + + return { + kind: DiffType.Submodule, + fullPath, + path, + url, + status, + oldSHA, + newSHA, + } +} + +async function buildDiff( buffer: Buffer, repository: Repository, file: FileChange, oldestCommitish: string, lineEndingsChange?: LineEndingsChange ): Promise { + if (file.status.submoduleStatus !== undefined) { + return buildSubmoduleDiff( + buffer, + repository, + file, + file.status.submoduleStatus + ) + } + if (!isValidBuffer(buffer)) { // the buffer's diff is too large to be renderable in the UI - return Promise.resolve({ kind: DiffType.Unrenderable }) + return { kind: DiffType.Unrenderable } } const diff = diffFromRawDiffOutput(buffer) @@ -507,7 +560,7 @@ function buildDiff( hasHiddenBidiChars: diff.hasHiddenBidiChars, } - return Promise.resolve(largeTextDiff) + return largeTextDiff } return convertDiff(repository, file, diff, oldestCommitish, lineEndingsChange) diff --git a/app/src/lib/git/remote.ts b/app/src/lib/git/remote.ts index f3b6ae2340..13de73ab5d 100644 --- a/app/src/lib/git/remote.ts +++ b/app/src/lib/git/remote.ts @@ -6,6 +6,7 @@ import { IRemote } from '../../models/remote' import { envForRemoteOperation } from './environment' import { IGitAccount } from '../../models/git-account' import { getSymbolicRef } from './refs' +import { gitNetworkArguments } from '.' /** * List the remotes, sorted alphabetically by `name`, for a repository. @@ -106,7 +107,7 @@ export async function updateRemoteHEAD( } await git( - ['remote', 'set-head', '-a', remote.name], + [...gitNetworkArguments(), 'remote', 'set-head', '-a', remote.name], repository.path, 'updateRemoteHEAD', options diff --git a/app/src/lib/git/status.ts b/app/src/lib/git/status.ts index b5008a8537..4038ece0f9 100644 --- a/app/src/lib/git/status.ts +++ b/app/src/lib/git/status.ts @@ -104,7 +104,10 @@ function parseConflictedState( conflictDetails.conflictCountsByPath.get(path) || 0, } } else { - return { kind: AppFileStatusKind.Conflicted, entry } + return { + kind: AppFileStatusKind.Conflicted, + entry, + } } } case UnmergedEntrySummary.BothModified: { @@ -140,18 +143,38 @@ function convertToAppStatus( if (entry.kind === 'ordinary') { switch (entry.type) { case 'added': - return { kind: AppFileStatusKind.New } + return { + kind: AppFileStatusKind.New, + submoduleStatus: entry.submoduleStatus, + } case 'modified': - return { kind: AppFileStatusKind.Modified } + return { + kind: AppFileStatusKind.Modified, + submoduleStatus: entry.submoduleStatus, + } case 'deleted': - return { kind: AppFileStatusKind.Deleted } + return { + kind: AppFileStatusKind.Deleted, + submoduleStatus: entry.submoduleStatus, + } } } else if (entry.kind === 'copied' && oldPath != null) { - return { kind: AppFileStatusKind.Copied, oldPath } + return { + kind: AppFileStatusKind.Copied, + oldPath, + submoduleStatus: entry.submoduleStatus, + } } else if (entry.kind === 'renamed' && oldPath != null) { - return { kind: AppFileStatusKind.Renamed, oldPath } + return { + kind: AppFileStatusKind.Renamed, + oldPath, + submoduleStatus: entry.submoduleStatus, + } } else if (entry.kind === 'untracked') { - return { kind: AppFileStatusKind.Untracked } + return { + kind: AppFileStatusKind.Untracked, + submoduleStatus: entry.submoduleStatus, + } } else if (entry.kind === 'conflicted') { return parseConflictedState(entry, path, conflictDetails) } @@ -270,7 +293,7 @@ function buildStatusMap( entry: IStatusEntry, conflictDetails: ConflictFilesDetails ): Map { - const status = mapStatus(entry.statusCode) + const status = mapStatus(entry.statusCode, entry.submoduleStatusCode) if (status.kind === 'ordinary') { // when a file is added in the index but then removed in the working @@ -300,7 +323,14 @@ function buildStatusMap( entry.oldPath ) - const selection = DiffSelection.fromInitialSelection(DiffSelectionType.All) + const initialSelectionType = + appStatus.kind === AppFileStatusKind.Modified && + appStatus.submoduleStatus !== undefined && + !appStatus.submoduleStatus.commitChanged + ? DiffSelectionType.None + : DiffSelectionType.All + + const selection = DiffSelection.fromInitialSelection(initialSelectionType) files.set( entry.path, diff --git a/app/src/lib/status-parser.ts b/app/src/lib/status-parser.ts index c642e97fec..4473df754f 100644 --- a/app/src/lib/status-parser.ts +++ b/app/src/lib/status-parser.ts @@ -1,8 +1,10 @@ import { FileEntry, GitStatusEntry, + SubmoduleStatus, UnmergedEntrySummary, } from '../models/status' +import { enableSubmoduleDiff } from './feature-flag' type StatusItem = IStatusHeader | IStatusEntry @@ -21,6 +23,9 @@ export interface IStatusEntry { /** The two character long status code */ readonly statusCode: string + /** The four character long submodule status code */ + readonly submoduleStatusCode: string + /** The original path in the case of a renamed file */ readonly oldPath?: string } @@ -102,7 +107,12 @@ function parseChangedEntry(field: string): IStatusEntry { throw new Error(`Failed to parse status line for changed entry`) } - return { kind: 'entry', statusCode: match[1], path: match[8] } + return { + kind: 'entry', + statusCode: match[1], + submoduleStatusCode: match[2], + path: match[8], + } } // 2 @@ -126,7 +136,13 @@ function parsedRenamedOrCopiedEntry( ) } - return { kind: 'entry', statusCode: match[1], oldPath, path: match[9] } + return { + kind: 'entry', + statusCode: match[1], + submoduleStatusCode: match[2], + oldPath, + path: match[9], + } } // u

@@ -144,6 +160,7 @@ function parseUnmergedEntry(field: string): IStatusEntry { return { kind: 'entry', statusCode: match[1], + submoduleStatusCode: match[2], path: match[10], } } @@ -155,198 +172,239 @@ function parseUntrackedEntry(field: string): IStatusEntry { // NOTE: We return ?? instead of ? here to play nice with mapStatus, // might want to consider changing this (and mapStatus) in the future. statusCode: '??', + submoduleStatusCode: '????', path, } } +function mapSubmoduleStatus( + submoduleStatusCode: string +): SubmoduleStatus | undefined { + if (!enableSubmoduleDiff() || !submoduleStatusCode.startsWith('S')) { + return undefined + } + + return { + commitChanged: submoduleStatusCode[1] === 'C', + modifiedChanges: submoduleStatusCode[2] === 'M', + untrackedChanges: submoduleStatusCode[3] === 'U', + } +} + /** * Map the raw status text from Git to a structure we can work with in the app. */ -export function mapStatus(status: string): FileEntry { - if (status === '??') { - return { kind: 'untracked' } +export function mapStatus( + statusCode: string, + submoduleStatusCode: string +): FileEntry { + const submoduleStatus = mapSubmoduleStatus(submoduleStatusCode) + + if (statusCode === '??') { + return { kind: 'untracked', submoduleStatus } } - if (status === '.M') { + if (statusCode === '.M') { return { kind: 'ordinary', type: 'modified', index: GitStatusEntry.Unchanged, workingTree: GitStatusEntry.Modified, + submoduleStatus, } } - if (status === 'M.') { + if (statusCode === 'M.') { return { kind: 'ordinary', type: 'modified', index: GitStatusEntry.Modified, workingTree: GitStatusEntry.Unchanged, + submoduleStatus, } } - if (status === '.A') { + if (statusCode === '.A') { return { kind: 'ordinary', type: 'added', index: GitStatusEntry.Unchanged, workingTree: GitStatusEntry.Added, + submoduleStatus, } } - if (status === 'A.') { + if (statusCode === 'A.') { return { kind: 'ordinary', type: 'added', index: GitStatusEntry.Added, workingTree: GitStatusEntry.Unchanged, + submoduleStatus, } } - if (status === '.D') { + if (statusCode === '.D') { return { kind: 'ordinary', type: 'deleted', index: GitStatusEntry.Unchanged, workingTree: GitStatusEntry.Deleted, + submoduleStatus, } } - if (status === 'D.') { + if (statusCode === 'D.') { return { kind: 'ordinary', type: 'deleted', index: GitStatusEntry.Deleted, workingTree: GitStatusEntry.Unchanged, + submoduleStatus, } } - if (status === 'R.') { + if (statusCode === 'R.') { return { kind: 'renamed', index: GitStatusEntry.Renamed, workingTree: GitStatusEntry.Unchanged, + submoduleStatus, } } - if (status === '.R') { + if (statusCode === '.R') { return { kind: 'renamed', index: GitStatusEntry.Unchanged, workingTree: GitStatusEntry.Renamed, + submoduleStatus, } } - if (status === 'C.') { + if (statusCode === 'C.') { return { kind: 'copied', index: GitStatusEntry.Copied, workingTree: GitStatusEntry.Unchanged, + submoduleStatus, } } - if (status === '.C') { + if (statusCode === '.C') { return { kind: 'copied', index: GitStatusEntry.Unchanged, workingTree: GitStatusEntry.Copied, + submoduleStatus, } } - if (status === 'AD') { + if (statusCode === 'AD') { return { kind: 'ordinary', type: 'added', index: GitStatusEntry.Added, workingTree: GitStatusEntry.Deleted, + submoduleStatus, } } - if (status === 'AM') { + if (statusCode === 'AM') { return { kind: 'ordinary', type: 'added', index: GitStatusEntry.Added, workingTree: GitStatusEntry.Modified, + submoduleStatus, } } - if (status === 'RM') { + if (statusCode === 'RM') { return { kind: 'renamed', index: GitStatusEntry.Renamed, workingTree: GitStatusEntry.Modified, + submoduleStatus, } } - if (status === 'RD') { + if (statusCode === 'RD') { return { kind: 'renamed', index: GitStatusEntry.Renamed, workingTree: GitStatusEntry.Deleted, + submoduleStatus, } } - if (status === 'DD') { + if (statusCode === 'DD') { return { kind: 'conflicted', action: UnmergedEntrySummary.BothDeleted, us: GitStatusEntry.Deleted, them: GitStatusEntry.Deleted, + submoduleStatus, } } - if (status === 'AU') { + if (statusCode === 'AU') { return { kind: 'conflicted', action: UnmergedEntrySummary.AddedByUs, us: GitStatusEntry.Added, them: GitStatusEntry.UpdatedButUnmerged, + submoduleStatus, } } - if (status === 'UD') { + if (statusCode === 'UD') { return { kind: 'conflicted', action: UnmergedEntrySummary.DeletedByThem, us: GitStatusEntry.UpdatedButUnmerged, them: GitStatusEntry.Deleted, + submoduleStatus, } } - if (status === 'UA') { + if (statusCode === 'UA') { return { kind: 'conflicted', action: UnmergedEntrySummary.AddedByThem, us: GitStatusEntry.UpdatedButUnmerged, them: GitStatusEntry.Added, + submoduleStatus, } } - if (status === 'DU') { + if (statusCode === 'DU') { return { kind: 'conflicted', action: UnmergedEntrySummary.DeletedByUs, us: GitStatusEntry.Deleted, them: GitStatusEntry.UpdatedButUnmerged, + submoduleStatus, } } - if (status === 'AA') { + if (statusCode === 'AA') { return { kind: 'conflicted', action: UnmergedEntrySummary.BothAdded, us: GitStatusEntry.Added, them: GitStatusEntry.Added, + submoduleStatus, } } - if (status === 'UU') { + if (statusCode === 'UU') { return { kind: 'conflicted', action: UnmergedEntrySummary.BothModified, us: GitStatusEntry.UpdatedButUnmerged, them: GitStatusEntry.UpdatedButUnmerged, + submoduleStatus, } } @@ -354,5 +412,6 @@ export function mapStatus(status: string): FileEntry { return { kind: 'ordinary', type: 'modified', + submoduleStatus, } } diff --git a/app/src/lib/stores/notifications-store.ts b/app/src/lib/stores/notifications-store.ts index 8d9303a0c3..e88b6922c4 100644 --- a/app/src/lib/stores/notifications-store.ts +++ b/app/src/lib/stores/notifications-store.ts @@ -16,7 +16,7 @@ import { AccountsStore } from './accounts-store' import { getCommit } from '../git' import { GitHubRepository } from '../../models/github-repository' import { PullRequestCoordinator } from './pull-request-coordinator' -import { Commit } from '../../models/commit' +import { Commit, shortenSHA } from '../../models/commit' import { AliveStore, DesktopAliveEvent, @@ -253,7 +253,7 @@ export class NotificationsStore { const pluralChecks = numberOfFailedChecks === 1 ? 'check was' : 'checks were' - const shortSHA = commitSHA.slice(0, 9) + const shortSHA = shortenSHA(commitSHA) const title = 'Pull Request checks failed' const body = `${pullRequest.title} #${pullRequest.pullRequestNumber} (${shortSHA})\n${numberOfFailedChecks} ${pluralChecks} not successful.` const onClick = () => { diff --git a/app/src/models/commit.ts b/app/src/models/commit.ts index 61e558a606..44a0d8c206 100644 --- a/app/src/models/commit.ts +++ b/app/src/models/commit.ts @@ -2,6 +2,11 @@ import { CommitIdentity } from './commit-identity' import { ITrailer, isCoAuthoredByTrailer } from '../lib/git/interpret-trailers' import { GitAuthor } from './git-author' +/** Shortens a given SHA. */ +export function shortenSHA(sha: string) { + return sha.slice(0, 9) +} + /** Grouping of information required to create a commit */ export interface ICommitContext { /** diff --git a/app/src/models/diff/diff-data.ts b/app/src/models/diff/diff-data.ts index 99cea42042..584a105800 100644 --- a/app/src/models/diff/diff-data.ts +++ b/app/src/models/diff/diff-data.ts @@ -1,5 +1,6 @@ import { DiffHunk } from './raw-diff' import { Image } from './image' +import { SubmoduleStatus } from '../status' /** * V8 has a limit on the size of string it can create, and unless we want to * trigger an unhandled exception we need to do the encoding conversion by hand @@ -87,6 +88,28 @@ export interface IBinaryDiff { readonly kind: DiffType.Binary } +export interface ISubmoduleDiff { + readonly kind: DiffType.Submodule + + /** Full path of the submodule */ + readonly fullPath: string + + /** Path of the repository within its container repository */ + readonly path: string + + /** URL of the submodule */ + readonly url: string + + /** Status of the submodule */ + readonly status: SubmoduleStatus + + /** Previous SHA of the submodule, or null if it hasn't changed */ + readonly oldSHA: string | null + + /** New SHA of the submodule, or null if it hasn't changed */ + readonly newSHA: string | null +} + export interface ILargeTextDiff extends ITextDiffData { readonly kind: DiffType.LargeText } @@ -100,5 +123,6 @@ export type IDiff = | ITextDiff | IImageDiff | IBinaryDiff + | ISubmoduleDiff | ILargeTextDiff | IUnrenderableDiff diff --git a/app/src/models/status.ts b/app/src/models/status.ts index 4189fb6994..1a40444d29 100644 --- a/app/src/models/status.ts +++ b/app/src/models/status.ts @@ -34,6 +34,7 @@ export type PlainFileStatus = { | AppFileStatusKind.New | AppFileStatusKind.Modified | AppFileStatusKind.Deleted + submoduleStatus?: SubmoduleStatus } /** @@ -46,6 +47,7 @@ export type PlainFileStatus = { export type CopiedOrRenamedFileStatus = { kind: AppFileStatusKind.Copied | AppFileStatusKind.Renamed oldPath: string + submoduleStatus?: SubmoduleStatus } /** @@ -56,6 +58,7 @@ export type ConflictsWithMarkers = { kind: AppFileStatusKind.Conflicted entry: TextConflictEntry conflictMarkerCount: number + submoduleStatus?: SubmoduleStatus } /** @@ -65,6 +68,7 @@ export type ConflictsWithMarkers = { export type ManualConflict = { kind: AppFileStatusKind.Conflicted entry: ManualConflictEntry + submoduleStatus?: SubmoduleStatus } /** Union of potential conflict scenarios the application should handle */ @@ -92,7 +96,10 @@ export function isManualConflict( } /** Denotes an untracked file in the working directory) */ -export type UntrackedFileStatus = { kind: AppFileStatusKind.Untracked } +export type UntrackedFileStatus = { + kind: AppFileStatusKind.Untracked + submoduleStatus?: SubmoduleStatus +} /** The union of potential states associated with a file change in Desktop */ export type AppFileStatus = @@ -101,6 +108,22 @@ export type AppFileStatus = | ConflictedFileStatus | UntrackedFileStatus +/** The status of a submodule */ +export type SubmoduleStatus = { + /** Whether or not the submodule is pointing to a different commit */ + readonly commitChanged: boolean + /** + * Whether or not the submodule contains modified changes that haven't been + * committed yet + */ + readonly modifiedChanges: boolean + /** + * Whether or not the submodule contains untracked changes that haven't been + * committed yet + */ + readonly untrackedChanges: boolean +} + /** The porcelain status for an ordinary changed entry */ type OrdinaryEntry = { readonly kind: 'ordinary' @@ -110,6 +133,8 @@ type OrdinaryEntry = { readonly index?: GitStatusEntry /** the status of the working tree for this entry (if known) */ readonly workingTree?: GitStatusEntry + /** the submodule status for this entry */ + readonly submoduleStatus?: SubmoduleStatus } /** The porcelain status for a renamed or copied entry */ @@ -119,6 +144,8 @@ type RenamedOrCopiedEntry = { readonly index?: GitStatusEntry /** the status of the working tree for this entry (if known) */ readonly workingTree?: GitStatusEntry + /** the submodule status for this entry */ + readonly submoduleStatus?: SubmoduleStatus } export enum UnmergedEntrySummary { @@ -149,13 +176,18 @@ type TextConflictDetails = type TextConflictEntry = { readonly kind: 'conflicted' + /** the submodule status for this entry */ + readonly submoduleStatus?: SubmoduleStatus } & TextConflictDetails /** * Valid Git index states where the user needs to choose one of `us` or `them` * in the app. */ -type ManualConflictDetails = +type ManualConflictDetails = { + /** the submodule status for this entry */ + readonly submoduleStatus?: SubmoduleStatus +} & ( | { readonly action: UnmergedEntrySummary.BothAdded readonly us: GitStatusEntry.Added @@ -191,9 +223,12 @@ type ManualConflictDetails = readonly us: GitStatusEntry.Deleted readonly them: GitStatusEntry.Deleted } +) type ManualConflictEntry = { readonly kind: 'conflicted' + /** the submodule status for this entry */ + readonly submoduleStatus?: SubmoduleStatus } & ManualConflictDetails /** The porcelain status for an unmerged entry */ @@ -202,6 +237,8 @@ export type UnmergedEntry = TextConflictEntry | ManualConflictEntry /** The porcelain status for an unmerged entry */ type UntrackedEntry = { readonly kind: 'untracked' + /** the submodule status for this entry */ + readonly submoduleStatus?: SubmoduleStatus } /** The union of possible entries from the git status */ diff --git a/app/src/ui/app-menu/app-menu-bar-button.tsx b/app/src/ui/app-menu/app-menu-bar-button.tsx index 65e89f1ab0..06a0642dff 100644 --- a/app/src/ui/app-menu/app-menu-bar-button.tsx +++ b/app/src/ui/app-menu/app-menu-bar-button.tsx @@ -209,6 +209,7 @@ export class AppMenuBarButton extends React.Component< highlightAccessKey={this.props.highlightMenuAccessKey} renderAcceleratorText={false} renderSubMenuArrow={false} + selected={false} /> ) diff --git a/app/src/ui/app-menu/app-menu.tsx b/app/src/ui/app-menu/app-menu.tsx index 36bf8c59cc..4957e03a54 100644 --- a/app/src/ui/app-menu/app-menu.tsx +++ b/app/src/ui/app-menu/app-menu.tsx @@ -166,11 +166,12 @@ export class AppMenu extends React.Component { } } - private onItemKeyDown = ( + private onPaneKeyDown = ( depth: number, - item: MenuItem, - event: React.KeyboardEvent + event: React.KeyboardEvent ) => { + const { selectedItem } = this.props.state[depth] + if (event.key === 'ArrowLeft' || event.key === 'Escape') { this.clearExpandCollapseTimer() @@ -191,9 +192,9 @@ export class AppMenu extends React.Component { this.clearExpandCollapseTimer() // Open the submenu and select the first item - if (item.type === 'submenuItem') { + if (selectedItem?.type === 'submenuItem') { this.props.dispatcher.setAppMenuState(menu => - menu.withOpenedMenu(item, true) + menu.withOpenedMenu(selectedItem, true) ) this.focusPane = depth + 1 event.preventDefault() @@ -222,6 +223,12 @@ export class AppMenu extends React.Component { }, expandCollapseTimeout) } + private onClearSelection = (depth: number) => { + this.props.dispatcher.setAppMenuState(appMenu => + appMenu.withDeselectedMenu(this.props.state[depth]) + ) + } + private onSelectionChanged = ( depth: number, item: MenuItem, @@ -280,12 +287,6 @@ export class AppMenu extends React.Component { } private renderMenuPane(depth: number, menu: IMenu): JSX.Element { - // NB: We use the menu id instead of depth as the key here to force - // a new MenuPane instance and List. This is because we used dynamic - // row heights and the react-virtualized Grid component isn't able to - // recompute row heights accurately. Without this row indices which - // previously held a separator item will retain that height and vice- - // versa. // If the menu doesn't have an id it's the root menu const key = menu.id || '@' const className = menu.id ? menuPaneClassNameFromId(menu.id) : undefined @@ -295,15 +296,15 @@ export class AppMenu extends React.Component { key={key} ref={this.onMenuPaneRef} className={className} - autoHeight={this.props.autoHeight} depth={depth} items={menu.items} selectedItem={menu.selectedItem} onItemClicked={this.onItemClicked} onMouseEnter={this.onPaneMouseEnter} - onItemKeyDown={this.onItemKeyDown} + onKeyDown={this.onPaneKeyDown} onSelectionChanged={this.onSelectionChanged} enableAccessKeyNavigation={this.props.enableAccessKeyNavigation} + onClearSelection={this.onClearSelection} /> ) } @@ -317,6 +318,7 @@ export class AppMenu extends React.Component { this.paneRefs = this.paneRefs.slice(0, panes.length) return ( + // eslint-disable-next-line jsx-a11y/no-static-element-interactions
{panes}
diff --git a/app/src/ui/app-menu/menu-list-item.tsx b/app/src/ui/app-menu/menu-list-item.tsx index aa7a7f3787..ddff84681c 100644 --- a/app/src/ui/app-menu/menu-list-item.tsx +++ b/app/src/ui/app-menu/menu-list-item.tsx @@ -34,6 +34,35 @@ interface IMenuListItemProps { * Defaults to true if not specified (i.e. undefined) */ readonly renderSubMenuArrow?: boolean + + /** + * Whether or not the menu item represented by this list item is the currently + * selected menu item. + */ + readonly selected: boolean + + /** Called when the user's pointer device enter the list item */ + readonly onMouseEnter?: ( + item: MenuItem, + event: React.MouseEvent + ) => void + /** Called when the user's pointer device leaves the list item */ + readonly onMouseLeave?: ( + item: MenuItem, + event: React.MouseEvent + ) => void + + /** Called when the user's pointer device clicks on the list item */ + readonly onClick?: ( + item: MenuItem, + event: React.MouseEvent + ) => void + + /** + * Whether the list item should steal focus when selected. Defaults to + * false. + */ + readonly focusOnSelection?: boolean } /** @@ -49,6 +78,8 @@ export function friendlyAcceleratorText(accelerator: string): string { } export class MenuListItem extends React.Component { + private wrapperRef = React.createRef() + private getIcon(item: MenuItem): JSX.Element | null { if (item.type === 'checkbox' && item.checked) { return @@ -59,6 +90,31 @@ export class MenuListItem extends React.Component { return null } + private onMouseEnter = (event: React.MouseEvent) => { + this.props.onMouseEnter?.(this.props.item, event) + } + + private onMouseLeave = (event: React.MouseEvent) => { + this.props.onMouseLeave?.(this.props.item, event) + } + + private onClick = (event: React.MouseEvent) => { + this.props.onClick?.(this.props.item, event) + } + + public componentDidMount() { + if (this.props.selected && this.props.focusOnSelection) { + this.wrapperRef.current?.focus() + } + } + + public componentDidUpdate(prevProps: IMenuListItemProps) { + const { focusOnSelection, selected } = this.props + if (focusOnSelection && selected && !prevProps.selected) { + this.wrapperRef.current?.focus() + } + } + public render() { const item = this.props.item @@ -83,19 +139,26 @@ export class MenuListItem extends React.Component { ) : null - const className = classNames( - 'menu-item', - { disabled: !item.enabled }, - { checkbox: item.type === 'checkbox' }, - { radio: item.type === 'radio' }, - { - checked: - (item.type === 'checkbox' || item.type === 'radio') && item.checked, - } - ) + const { type } = item + + const className = classNames('menu-item', { + disabled: !item.enabled, + checkbox: type === 'checkbox', + radio: type === 'radio', + checked: (type === 'checkbox' || type === 'radio') && item.checked, + selected: this.props.selected, + }) return ( -
+ // eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/interactive-supports-focus +
{this.getIcon(item)}
void /** - * A callback for when a keyboard key is pressed on a menu item. Note that - * this only picks up on keyboard events received by a MenuItem and does - * not cover keyboard events received on the MenuPane component itself. + * Called when the user presses down on a key while focused on, or within, the + * menu pane. Consumers should inspect isDefaultPrevented to determine whether + * the event was handled by the menu pane or not. */ - readonly onItemKeyDown?: ( + readonly onKeyDown?: ( depth: number, - item: MenuItem, - event: React.KeyboardEvent + event: React.KeyboardEvent ) => void /** @@ -77,115 +85,52 @@ interface IMenuPaneProps { readonly enableAccessKeyNavigation: boolean /** - * If true the MenuPane only takes up as much vertical space needed to - * show all menu items. This does not affect maximum height, i.e. if the - * visible menu items takes up more space than what is available the menu - * will still overflow and be scrollable. - * - * @default false + * Called to deselect the currently selected menu item (if any). This + * will be called when the user's pointer device leaves a menu item. */ - readonly autoHeight?: boolean + readonly onClearSelection: (depth: number) => void } -interface IMenuPaneState { - /** - * A list of visible menu items that is to be rendered. This is a derivative - * of the props items with invisible items filtered out. - */ - readonly items: ReadonlyArray - - /** The selected row index or -1 if no selection exists. */ - readonly selectedIndex: number -} - -const RowHeight = 30 -const SeparatorRowHeight = 10 - -function getSelectedIndex( - selectedItem: MenuItem | undefined, - items: ReadonlyArray -) { - return selectedItem ? items.findIndex(i => i.id === selectedItem.id) : -1 -} - -export function getListHeight(menuItems: ReadonlyArray) { - return menuItems.reduce((acc, item) => acc + getRowHeight(item), 0) -} - -export function getRowHeight(item: MenuItem) { - if (!item.visible) { - return 0 - } - - return item.type === 'separator' ? SeparatorRowHeight : RowHeight -} - -/** - * Creates a menu pane state given props. This is intentionally not - * an instance member in order to avoid mistakenly using any other - * input data or state than the received props. - */ -function createState(props: IMenuPaneProps): IMenuPaneState { - const items = new Array() - const selectedItem = props.selectedItem - - let selectedIndex = -1 - - // Filter out all invisible items and maintain the correct - // selected index (if possible) - for (let i = 0; i < props.items.length; i++) { - const item = props.items[i] - - if (item.visible) { - items.push(item) - if (item === selectedItem) { - selectedIndex = items.length - 1 - } - } - } - - return { items, selectedIndex } -} - -export class MenuPane extends React.Component { - private list: List | null = null - - public constructor(props: IMenuPaneProps) { - super(props) - this.state = createState(props) - } - - public componentWillReceiveProps(nextProps: IMenuPaneProps) { - // No need to recreate the filtered list if it hasn't changed, - // we only have to update the selected item - if (this.props.items === nextProps.items) { - // Has the selection changed? - if (this.props.selectedItem !== nextProps.selectedItem) { - const selectedIndex = getSelectedIndex( - nextProps.selectedItem, - this.state.items - ) - this.setState({ selectedIndex }) - } - } else { - this.setState(createState(nextProps)) - } - } - - private onRowClick = (row: number, source: ClickSource) => { - const item = this.state.items[row] +export class MenuPane extends React.Component { + private paneRef = React.createRef() + private onRowClick = ( + item: MenuItem, + event: React.MouseEvent + ) => { if (item.type !== 'separator' && item.enabled) { + const source: IMouseClickSource = { kind: 'mouseclick', event } this.props.onItemClicked(this.props.depth, item, source) } } - private onSelectedRowChanged = (row: number, source: SelectionSource) => { - const item = this.state.items[row] - this.props.onSelectionChanged(this.props.depth, item, source) + private tryMoveSelection( + direction: 'up' | 'down' | 'first' | 'last', + source: ClickSource + ) { + const { items, selectedItem } = this.props + const row = selectedItem ? items.indexOf(selectedItem) : -1 + const count = items.length + const selectable = (ix: number) => items[ix] && itemIsSelectable(items[ix]) + + let ix: number | null = null + + if (direction === 'up' || direction === 'down') { + ix = findNextSelectableRow(count, { direction, row }, selectable) + } else if (direction === 'first' || direction === 'last') { + const d = direction === 'first' ? 'up' : 'down' + ix = findLastSelectableRow(d, count, selectable) + } + + if (ix !== null && items[ix] !== undefined) { + this.props.onSelectionChanged(this.props.depth, items[ix], source) + return true + } + + return false } - private onKeyDown = (event: React.KeyboardEvent) => { + private onKeyDown = (event: React.KeyboardEvent) => { if (event.defaultPrevented) { return } @@ -195,103 +140,115 @@ export class MenuPane extends React.Component { return } + const source: IKeyboardSource = { kind: 'keyboard', event } + const { selectedItem } = this.props + const { key } = event + + if (isSupportedKey(key)) { + event.preventDefault() + + if (key === 'ArrowUp' || key === 'ArrowDown') { + this.tryMoveSelection(key === 'ArrowUp' ? 'up' : 'down', source) + } else if (key === 'Home' || key === 'End') { + const direction = key === 'Home' ? 'first' : 'last' + this.tryMoveSelection(direction, source) + } else if (key === 'Enter' || key === ' ') { + if (selectedItem !== undefined) { + this.props.onItemClicked(this.props.depth, selectedItem, source) + } + } else { + assertNever(key, 'Unsupported key') + } + } + // If we weren't opened with the Alt key we ignore key presses other than // arrow keys and Enter/Space etc. - if (!this.props.enableAccessKeyNavigation) { - return + if (this.props.enableAccessKeyNavigation) { + // At this point the list will already have intercepted any arrow keys + // and the list items themselves will have caught Enter/Space + const item = findItemByAccessKey(event.key, this.props.items) + if (item && itemIsSelectable(item)) { + event.preventDefault() + this.props.onSelectionChanged(this.props.depth, item, { + kind: 'keyboard', + event: event, + }) + this.props.onItemClicked(this.props.depth, item, { + kind: 'keyboard', + event: event, + }) + } } - // At this point the list will already have intercepted any arrow keys - // and the list items themselves will have caught Enter/Space - const item = findItemByAccessKey(event.key, this.state.items) - if (item && itemIsSelectable(item)) { - event.preventDefault() - this.props.onSelectionChanged(this.props.depth, item, { - kind: 'keyboard', - event: event, - }) - this.props.onItemClicked(this.props.depth, item, { - kind: 'keyboard', - event: event, - }) - } - } - - private onRowKeyDown = (row: number, event: React.KeyboardEvent) => { - if (this.props.onItemKeyDown) { - const item = this.state.items[row] - this.props.onItemKeyDown(this.props.depth, item, event) - } - } - - private canSelectRow = (row: number) => { - const item = this.state.items[row] - return itemIsSelectable(item) - } - - private onListRef = (list: List | null) => { - this.list = list + this.props.onKeyDown?.(this.props.depth, event) } private onMouseEnter = (event: React.MouseEvent) => { - if (this.props.onMouseEnter) { - this.props.onMouseEnter(this.props.depth) + this.props.onMouseEnter?.(this.props.depth) + } + + private onRowMouseEnter = ( + item: MenuItem, + event: React.MouseEvent + ) => { + if (itemIsSelectable(item)) { + const source: IHoverSource = { kind: 'hover', event } + this.props.onSelectionChanged(this.props.depth, item, source) } } - private renderMenuItem = (row: number) => { - const item = this.state.items[row] - - return ( - - ) - } - - private rowHeight = (info: { index: number }) => { - const item = this.state.items[info.index] - return item.type === 'separator' ? SeparatorRowHeight : RowHeight + private onRowMouseLeave = ( + item: MenuItem, + event: React.MouseEvent + ) => { + if (this.props.selectedItem === item) { + this.props.onClearSelection(this.props.depth) + } } public render(): JSX.Element { - const style: React.CSSProperties = - this.props.autoHeight === true - ? { height: getListHeight(this.props.items) + 5, maxHeight: '100%' } - : {} - const className = classNames('menu-pane', this.props.className) return ( + // eslint-disable-next-line jsx-a11y/no-static-element-interactions
- + {this.props.items + .filter(x => x.visible) + .map((item, ix) => ( + + ))}
) } public focus() { - if (this.list) { - this.list.focus() - } + this.paneRef.current?.focus() } } + +const supportedKeys = [ + 'ArrowUp', + 'ArrowDown', + 'Home', + 'End', + 'Enter', + ' ', +] as const +const isSupportedKey = (key: string): key is typeof supportedKeys[number] => + (supportedKeys as readonly string[]).includes(key) diff --git a/app/src/ui/autocompletion/emoji-autocompletion-provider.tsx b/app/src/ui/autocompletion/emoji-autocompletion-provider.tsx index 64a0c436fd..c17146e31b 100644 --- a/app/src/ui/autocompletion/emoji-autocompletion-provider.tsx +++ b/app/src/ui/autocompletion/emoji-autocompletion-provider.tsx @@ -90,7 +90,7 @@ export class EmojiAutocompletionProvider return (
- + {emoji} {this.renderHighlightedTitle(hit)}
) diff --git a/app/src/ui/banners/banner.tsx b/app/src/ui/banners/banner.tsx index 016dbafb16..3d5ad042c5 100644 --- a/app/src/ui/banners/banner.tsx +++ b/app/src/ui/banners/banner.tsx @@ -1,3 +1,6 @@ +/* eslint-disable jsx-a11y/no-static-element-interactions */ +/* eslint-disable jsx-a11y/click-events-have-key-events */ +/* eslint-disable jsx-a11y/anchor-is-valid */ import * as React from 'react' import { Octicon } from '../octicons' import * as OcticonSymbol from '../octicons/octicons.generated' diff --git a/app/src/ui/branches/branch-list-item.tsx b/app/src/ui/branches/branch-list-item.tsx index 27dc741f10..e38a3f8e3b 100644 --- a/app/src/ui/branches/branch-list-item.tsx +++ b/app/src/ui/branches/branch-list-item.tsx @@ -131,6 +131,7 @@ export class BranchListItem extends React.Component< }) return ( + // eslint-disable-next-line jsx-a11y/no-static-element-interactions
{ if (this.props.canCreateNewBranch) { return (
- +
Sorry, I can't find that branch
diff --git a/app/src/ui/branches/no-pull-requests.tsx b/app/src/ui/branches/no-pull-requests.tsx index 2ff762b621..ab4d870d76 100644 --- a/app/src/ui/branches/no-pull-requests.tsx +++ b/app/src/ui/branches/no-pull-requests.tsx @@ -33,7 +33,7 @@ export class NoPullRequests extends React.Component { public render() { return (
- + {this.renderTitle()} {this.renderCallToAction()}
diff --git a/app/src/ui/branches/pull-request-badge.tsx b/app/src/ui/branches/pull-request-badge.tsx index 6ba5252ca7..80ccb31036 100644 --- a/app/src/ui/branches/pull-request-badge.tsx +++ b/app/src/ui/branches/pull-request-badge.tsx @@ -77,6 +77,7 @@ export class PullRequestBadge extends React.Component< public render() { const ref = getPullRequestCommitRef(this.props.number) return ( + // eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions
#{this.props.number} {this.renderDecorator()} - + {this.renderDiffOptions()} + ) + } + private renderDecorator() { const diff = this.props.diff diff --git a/app/src/ui/changes/changed-file.tsx b/app/src/ui/changes/changed-file.tsx index 39372f84a8..40b04dc073 100644 --- a/app/src/ui/changes/changed-file.tsx +++ b/app/src/ui/changes/changed-file.tsx @@ -6,12 +6,14 @@ import { Checkbox, CheckboxValue } from '../lib/checkbox' import { mapStatus } from '../../lib/status' import { WorkingDirectoryFileChange } from '../../models/status' import { TooltipDirection } from '../lib/tooltip' +import { TooltippedContent } from '../lib/tooltipped-content' interface IChangedFileProps { readonly file: WorkingDirectoryFileChange readonly include: boolean | null readonly availableWidth: number readonly disableSelection: boolean + readonly checkboxTooltip?: string readonly onIncludeChanged: (path: string, include: boolean) => void /** Callback called when user right-clicks on an item */ @@ -39,7 +41,9 @@ export class ChangedFile extends React.Component { } public render() { - const { status, path } = this.props.file + const { file, availableWidth, disableSelection, checkboxTooltip } = + this.props + const { status, path } = file const fileStatus = mapStatus(status) const listItemPadding = 10 * 2 @@ -48,7 +52,7 @@ export class ChangedFile extends React.Component { const filePadding = 5 const availablePathWidth = - this.props.availableWidth - + availableWidth - listItemPadding - checkboxWidth - filePadding - @@ -56,15 +60,21 @@ export class ChangedFile extends React.Component { return (
- + + + ) } @@ -852,6 +873,7 @@ export class ChangesList extends React.Component< ) return ( + // eslint-disable-next-line jsx-a11y/role-supports-aria-props - - ) - } - - private onCopyShaButtonClick = (e: React.MouseEvent) => { - e.preventDefault() - clipboard.writeText(this.getShaRef()) - } - private renderChangedFilesDescription = () => { const fileCount = this.props.changesetData.files.length const filesPlural = fileCount === 1 ? 'file' : 'files' diff --git a/app/src/ui/history/selected-commits.tsx b/app/src/ui/history/selected-commits.tsx index 256c28a3e1..3fbfd48eff 100644 --- a/app/src/ui/history/selected-commits.tsx +++ b/app/src/ui/history/selected-commits.tsx @@ -316,7 +316,7 @@ export class SelectedCommits extends React.Component< return (
- +

Unable to display diff when multiple{' '} @@ -441,7 +441,7 @@ function NoCommitSelected() { return (

- + No commit selected
) diff --git a/app/src/ui/history/unreachable-commits-dialog.tsx b/app/src/ui/history/unreachable-commits-dialog.tsx index 7919e09f11..db8edf205e 100644 --- a/app/src/ui/history/unreachable-commits-dialog.tsx +++ b/app/src/ui/history/unreachable-commits-dialog.tsx @@ -4,6 +4,7 @@ import { TabBar } from '../tab-bar' import { OkCancelButtonGroup } from '../dialog/ok-cancel-button-group' import { Commit } from '../../models/commit' import { CommitList } from './commit-list' +import { LinkButton } from '../lib/link-button' export enum UnreachableCommitsTab { Unreachable, @@ -127,7 +128,10 @@ export class UnreachableCommitsDialog extends React.Component< {this.state.selectedTab === UnreachableCommitsTab.Unreachable ? 'not' : ''}{' '} - in the ancestry path of the most recent commit in your selection. + in the ancestry path of the most recent commit in your selection.{' '} + + Learn more. +
) } diff --git a/app/src/ui/lib/authentication-form.tsx b/app/src/ui/lib/authentication-form.tsx index 6418aaaaba..5d3bc20e50 100644 --- a/app/src/ui/lib/authentication-form.tsx +++ b/app/src/ui/lib/authentication-form.tsx @@ -104,6 +104,7 @@ export class AuthenticationForm extends React.Component< diff --git a/app/src/ui/lib/draggable.tsx b/app/src/ui/lib/draggable.tsx index 30250e3f74..5357e927d5 100644 --- a/app/src/ui/lib/draggable.tsx +++ b/app/src/ui/lib/draggable.tsx @@ -174,6 +174,7 @@ export class Draggable extends React.Component { public render() { return ( + // eslint-disable-next-line jsx-a11y/no-static-element-interactions
{this.props.children}
diff --git a/app/src/ui/lib/enterprise-server-entry.tsx b/app/src/ui/lib/enterprise-server-entry.tsx index 13adb95c10..3232faaeb2 100644 --- a/app/src/ui/lib/enterprise-server-entry.tsx +++ b/app/src/ui/lib/enterprise-server-entry.tsx @@ -55,6 +55,7 @@ export class EnterpriseServerEntry extends React.Component<
extends React.Component< { const { title } = this.props return ( + // eslint-disable-next-line jsx-a11y/mouse-events-have-key-events { const style = { ...this.props.style, width: '100%' } return ( + // eslint-disable-next-line jsx-a11y/mouse-events-have-key-events
{ const role = this.props.ariaMode === 'menu' ? 'menu' : 'listbox' return ( + // eslint-disable-next-line jsx-a11y/aria-activedescendant-has-tabindex
{ > { ? availableWidth / 2 - ResizeArrowPadding : undefined return ( + // eslint-disable-next-line jsx-a11y/label-has-associated-control