From a5c5ddc8edc660ee8ecff1022bd011b875d75fae Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Tue, 30 Mar 2021 19:20:01 +0200 Subject: [PATCH 1/5] Fix selection on expanded diffs --- app/src/ui/diff/diff-explorer.ts | 46 +++++++++++++++++++++++ app/src/ui/diff/side-by-side-diff.tsx | 14 +++++-- app/src/ui/diff/text-diff.tsx | 53 +++++++++++++++++++++------ 3 files changed, 98 insertions(+), 15 deletions(-) diff --git a/app/src/ui/diff/diff-explorer.ts b/app/src/ui/diff/diff-explorer.ts index 33fa94d497..507cc41aae 100644 --- a/app/src/ui/diff/diff-explorer.ts +++ b/app/src/ui/diff/diff-explorer.ts @@ -94,6 +94,52 @@ export function lineNumberForDiffLine( return -1 } +/** + * For the given row in the diff, determine the range of elements that + * should be displayed as interactive, as a hunk is not granular enough. + * The values in the returned range are mapped to lines in the original diff, + * in case the current diff has been partially expanded. + */ +export function findInteractiveOriginalDiffRange( + hunks: ReadonlyArray, + index: number +): IDiffRange | null { + const range = findInteractiveDiffRange(hunks, index) + + if (range === null) { + return null + } + + const from = getLineInOriginalDiff(hunks, range.from) + const to = getLineInOriginalDiff(hunks, range.to) + + if (from === null || to === null) { + return null + } + + return { + ...range, + from, + to, + } +} + +/** + * Utility function to get the line number in the original line from a given + * line number in the current text diff (which might be expanded). + */ +export function getLineInOriginalDiff( + hunks: ReadonlyArray, + index: number +) { + const diffLine = diffLineForIndex(hunks, index) + if (diffLine === null) { + return null + } + + return diffLine.originalLineNumber +} + /** * For the given row in the diff, determine the range of elements that * should be displayed as interactive, as a hunk is not granular enough diff --git a/app/src/ui/diff/side-by-side-diff.tsx b/app/src/ui/diff/side-by-side-diff.tsx index fd581bd4a3..c866fba3fb 100644 --- a/app/src/ui/diff/side-by-side-diff.tsx +++ b/app/src/ui/diff/side-by-side-diff.tsx @@ -28,7 +28,10 @@ import { } from 'react-virtualized' import { SideBySideDiffRow } from './side-by-side-diff-row' import memoize from 'memoize-one' -import { findInteractiveDiffRange, DiffRangeType } from './diff-explorer' +import { + findInteractiveOriginalDiffRange, + DiffRangeType, +} from './diff-explorer' import { ChangedFile, DiffRow, @@ -589,7 +592,7 @@ export class SideBySideDiff extends React.Component< const selection = this.getSelection() if (selection !== undefined) { - const range = findInteractiveDiffRange(diff.hunks, hunkStartLine) + const range = findInteractiveOriginalDiffRange(diff.hunks, hunkStartLine) if (range !== null) { const { from, to } = range const sel = selection.withRangeSelection(from, to - from + 1, select) @@ -629,7 +632,7 @@ export class SideBySideDiff extends React.Component< return } - const range = findInteractiveDiffRange(diff.hunks, diffLineNumber) + const range = findInteractiveOriginalDiffRange(diff.hunks, diffLineNumber) if (range === null || range.type === null) { return } @@ -656,7 +659,10 @@ export class SideBySideDiff extends React.Component< return } - const range = findInteractiveDiffRange(this.props.diff.hunks, hunkStartLine) + const range = findInteractiveOriginalDiffRange( + this.props.diff.hunks, + hunkStartLine + ) if (range === null || range.type === null) { return } diff --git a/app/src/ui/diff/text-diff.tsx b/app/src/ui/diff/text-diff.tsx index fd3670c204..084c9ed2fe 100644 --- a/app/src/ui/diff/text-diff.tsx +++ b/app/src/ui/diff/text-diff.tsx @@ -21,10 +21,11 @@ import { DiffSyntaxMode, IDiffSyntaxModeSpec } from './diff-syntax-mode' import { CodeMirrorHost } from './code-mirror-host' import { diffLineForIndex, - findInteractiveDiffRange, + findInteractiveOriginalDiffRange, lineNumberForDiffLine, DiffRangeType, diffLineInfoForIndex, + getLineInOriginalDiff, } from './diff-explorer' import { @@ -453,10 +454,15 @@ export class TextDiff extends React.Component { this.cancelSelection() } - const isSelected = !file.selection.isSelected(index) + const indexInOriginalDiff = getLineInOriginalDiff(hunks, index) + if (indexInOriginalDiff === null) { + return + } + + const isSelected = !file.selection.isSelected(indexInOriginalDiff) if (kind === 'hunk') { - const range = findInteractiveDiffRange(hunks, index) + const range = findInteractiveOriginalDiffRange(hunks, index) if (!range) { console.error('unable to find range for given line in diff') return @@ -465,7 +471,12 @@ export class TextDiff extends React.Component { const { from, to } = range this.selection = { isSelected, from, to, kind } } else if (kind === 'range') { - this.selection = { isSelected, from: index, to: index, kind } + this.selection = { + isSelected, + from: indexInOriginalDiff, + to: indexInOriginalDiff, + kind, + } document.addEventListener('mousemove', this.onDocumentMouseMove) } else { assertNever(kind, `Unknown selection kind ${kind}`) @@ -496,9 +507,15 @@ export class TextDiff extends React.Component { // pointer is placed underneath the last line so we clamp it // to the range of valid values. const max = Math.max(0, this.codeMirror.getDoc().lineCount() - 1) - const to = clamp(this.codeMirror.lineAtHeight(ev.y), 0, max) + const index = clamp(this.codeMirror.lineAtHeight(ev.y), 0, max) - this.codeMirror.scrollIntoView({ line: to, ch: 0 }) + this.codeMirror.scrollIntoView({ line: index, ch: 0 }) + + const to = getLineInOriginalDiff(this.state.diff.hunks, index) + + if (to === null) { + return + } if (to !== this.selection.to) { this.selection = { ...this.selection, to } @@ -526,11 +543,21 @@ export class TextDiff extends React.Component { // we need to make sure the user is still within that hunk handle // section and in the correct range. if (this.selection.kind === 'hunk') { + const index = this.codeMirror.lineAtHeight(ev.y) + const indexInOriginalDiff = getLineInOriginalDiff( + this.state.diff.hunks, + index + ) + if (indexInOriginalDiff === null) { + return + } + // Is the pointer over the same range (i.e hunk) that the // selection was originally started from? if ( + indexInOriginalDiff === null || !targetHasClass(ev.target, 'hunk-handle') || - !inSelection(this.selection, this.codeMirror.lineAtHeight(ev.y)) + !inSelection(this.selection, indexInOriginalDiff) ) { return this.cancelSelection() } @@ -665,7 +692,10 @@ export class TextDiff extends React.Component { return null } - const range = findInteractiveDiffRange(this.state.diff.hunks, lineNumber) + const range = findInteractiveOriginalDiffRange( + this.state.diff.hunks, + lineNumber + ) if (range === null) { return null } @@ -1189,20 +1219,21 @@ export class TextDiff extends React.Component { return } const lineNumber = this.codeMirror.lineAtHeight(ev.y) - - const diffLine = diffLineForIndex(this.state.diff.hunks, lineNumber) + const hunks = this.state.diff.hunks + const diffLine = diffLineForIndex(hunks, lineNumber) if (!diffLine || !diffLine.isIncludeableLine()) { return } - const range = findInteractiveDiffRange(this.state.diff.hunks, lineNumber) + const range = findInteractiveOriginalDiffRange(hunks, lineNumber) if (range === null) { return } const { from, to } = range + this.hunkHighlightRange = { from, to, kind: 'hunk', isSelected: false } this.updateViewport() } From 2eec759a88bf5590878fdf27fa94be53a1e83040 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Thu, 1 Apr 2021 09:23:21 -0400 Subject: [PATCH 2/5] Add a way to switch between English and system langauge --- app/src/ui/main-process-proxy.ts | 48 ++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/app/src/ui/main-process-proxy.ts b/app/src/ui/main-process-proxy.ts index 7925e2fe36..9add5036b0 100644 --- a/app/src/ui/main-process-proxy.ts +++ b/app/src/ui/main-process-proxy.ts @@ -118,9 +118,57 @@ function mergeDeferredContextMenuItems( }) } + if (!__DARWIN__) { + // NOTE: "On macOS as we use the native APIs there is no way to set the + // language that the spellchecker uses" -- electron docs Therefore, we are + // only allowing setting to English for non-mac machines. + const spellCheckLanguageItem = getSpellCheckLanguageMenuItem( + webContents.session + ) + if (spellCheckLanguageItem !== null) { + items.push(spellCheckLanguageItem) + } + } + showContextualMenu(items, false) } +/** + * Method to get a menu item to give user the option to use English or their + * system language. + * + * If system language is english, it returns null. If spellchecker is not set to + * english, it returns item that can set it to English. If spellchecker is set + * to english, it returns the item that can set it to their system language. + */ +function getSpellCheckLanguageMenuItem( + session: Electron.session +): IMenuItem | null { + const userLanguageCode = remote.app.getLocale() + const englishLanguageCode = 'en-US' + if (userLanguageCode === englishLanguageCode) { + return null + } + + const currentLanguageCodes = session.getSpellCheckerLanguages() + + const languageCode = + currentLanguageCodes.includes(englishLanguageCode) && + !currentLanguageCodes.includes(userLanguageCode) + ? userLanguageCode + : englishLanguageCode + + const label = + languageCode === englishLanguageCode + ? 'Set spellcheck to English' + : 'Set spellcheck to system language' + + return { + label, + action: () => session.setSpellCheckerLanguages([languageCode]), + } +} + /** Show the given menu items in a contextual menu. */ export async function showContextualMenu( items: ReadonlyArray, From b16aaaef5f176b095ca7840275881b8bf142e097 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Thu, 1 Apr 2021 09:41:50 -0400 Subject: [PATCH 3/5] better wording --- app/src/ui/main-process-proxy.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/ui/main-process-proxy.ts b/app/src/ui/main-process-proxy.ts index 9add5036b0..6c41277a71 100644 --- a/app/src/ui/main-process-proxy.ts +++ b/app/src/ui/main-process-proxy.ts @@ -150,11 +150,11 @@ function getSpellCheckLanguageMenuItem( return null } - const currentLanguageCodes = session.getSpellCheckerLanguages() + const spellcheckLanguageCodes = session.getSpellCheckerLanguages() const languageCode = - currentLanguageCodes.includes(englishLanguageCode) && - !currentLanguageCodes.includes(userLanguageCode) + spellcheckLanguageCodes.includes(englishLanguageCode) && + !spellcheckLanguageCodes.includes(userLanguageCode) ? userLanguageCode : englishLanguageCode From e134f09134772d1ec47ff59d873b1ada89a6d0e2 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Thu, 1 Apr 2021 10:04:42 -0400 Subject: [PATCH 4/5] Make sure use can get back to English if electron is not set to English but system is --- app/src/ui/main-process-proxy.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/app/src/ui/main-process-proxy.ts b/app/src/ui/main-process-proxy.ts index 9add5036b0..50924ba776 100644 --- a/app/src/ui/main-process-proxy.ts +++ b/app/src/ui/main-process-proxy.ts @@ -146,15 +146,18 @@ function getSpellCheckLanguageMenuItem( ): IMenuItem | null { const userLanguageCode = remote.app.getLocale() const englishLanguageCode = 'en-US' - if (userLanguageCode === englishLanguageCode) { + const spellcheckLanguageCodes = session.getSpellCheckerLanguages() + + if ( + userLanguageCode === englishLanguageCode && + spellcheckLanguageCodes.includes(englishLanguageCode) + ) { return null } - const currentLanguageCodes = session.getSpellCheckerLanguages() - const languageCode = - currentLanguageCodes.includes(englishLanguageCode) && - !currentLanguageCodes.includes(userLanguageCode) + spellcheckLanguageCodes.includes(englishLanguageCode) && + !spellcheckLanguageCodes.includes(userLanguageCode) ? userLanguageCode : englishLanguageCode From 3ba4985e4f0807fb30511d2a63bd0d34d0acfdca Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Mon, 5 Apr 2021 07:06:28 -0400 Subject: [PATCH 5/5] Add ending drag on pressing Escape --- app/src/ui/lib/draggable.tsx | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/app/src/ui/lib/draggable.tsx b/app/src/ui/lib/draggable.tsx index f2de05adb3..4d36fac150 100644 --- a/app/src/ui/lib/draggable.tsx +++ b/app/src/ui/lib/draggable.tsx @@ -33,7 +33,8 @@ interface IDraggableProps { } export class Draggable extends React.Component { - private dragStarted: boolean = false + private hasDragStarted: boolean = false + private hasDragEnded: boolean = false private dragElement: HTMLElement | null = null private elemBelow: Element | null = null // Default offset to place the cursor slightly above the top left corner of @@ -41,7 +42,6 @@ export class Draggable extends React.Component { // dragElement then elemBelow will always return the dragElement and cannot // detect drop targets or scroll elements. private verticalOffset: number = __DARWIN__ ? 32 : 15 - private hasMouseUpOccurred = false public componentDidMount() { this.dragElement = document.getElementById('dragElement') @@ -58,7 +58,7 @@ export class Draggable extends React.Component { } private initializeDrag(): void { - this.dragStarted = false + this.hasDragStarted = false this.elemBelow = null } @@ -72,10 +72,10 @@ export class Draggable extends React.Component { if (!this.canDragCommit(event)) { return } - this.hasMouseUpOccurred = false - document.onmouseup = this.onMouseUp + this.hasDragEnded = false + document.onmouseup = this.handleDragEndEvent await sleep(100) - if (this.hasMouseUpOccurred) { + if (this.hasDragEnded) { return } @@ -91,16 +91,17 @@ export class Draggable extends React.Component { * just clicking a draggable element. */ private onMouseMove = (moveEvent: MouseEvent) => { - if (this.hasMouseUpOccurred) { + if (this.hasDragEnded) { this.onDragEnd() return } // start drag - if (!this.dragStarted) { + if (!this.hasDragStarted) { this.props.onRenderDragElement() this.props.onDragStart() dragAndDropManager.dragStarted() - this.dragStarted = true + this.hasDragStarted = true + window.addEventListener('keyup', this.onKeyUp) } // move drag element where mouse is @@ -126,12 +127,20 @@ export class Draggable extends React.Component { /** * End a drag event */ - private onMouseUp = () => { - this.hasMouseUpOccurred = true - if (this.dragStarted) { + private handleDragEndEvent = () => { + this.hasDragEnded = true + if (this.hasDragStarted) { this.onDragEnd() } document.onmouseup = null + window.removeEventListener('keyup', this.onKeyUp) + } + + private onKeyUp = (event: KeyboardEvent) => { + if (event.key !== 'Escape') { + return + } + this.handleDragEndEvent() } private onDragEnd(): void {