Merge pull request #18201 from desktop/add-check-all-semantic-control

Add Diff Selectable Line Group Check All: Add check all visual and semantic control
This commit is contained in:
tidy-dev 2024-02-27 12:35:39 -05:00 committed by GitHub
commit 38a1a96b5a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 235 additions and 63 deletions

View file

@ -60,12 +60,29 @@ export interface IRowSelectableGroup {
/**
* The selection state of the group - 'All', 'Partial', or 'None'
*/
selectionState: DiffSelectionType | null
selectionState: DiffSelectionType
/** The group's diff type, all 'added', all 'deleted', or a mix = 'modified */
diffType: DiffRowType
/**
* The height of the rows in the group
*/
height: number
/**
* The line numbers associated with the group
*/
lineNumbers: ReadonlyArray<number>
/**
* The line numbers identifiers associated with the group.
*
* If the line numbers are [4, 5, 6] then the lineNumbersIdentifiers could be
* something like [`4-before`, `4-after`, `5-after`, `6-after`] as the line
* number is not unique without the "before" or "after" suffix
*/
lineNumbersIdentifiers: ReadonlyArray<string>
}
interface ISideBySideDiffRowProps {
@ -214,10 +231,15 @@ export class SideBySideDiffRow extends React.Component<
}
public render() {
const { row, showSideBySideDiff, beforeClassNames, afterClassNames } =
this.props
const {
row,
showSideBySideDiff,
beforeClassNames,
afterClassNames,
isDiffSelectable,
} = this.props
const baseRowClasses = classNames('row', {
'has-check-all-control': enableGroupDiffCheckmarks(),
'has-check-all-control': enableGroupDiffCheckmarks() && isDiffSelectable,
})
const beforeClasses = classNames('before', ...beforeClassNames)
const afterClasses = classNames('after', ...afterClassNames)
@ -539,8 +561,20 @@ export class SideBySideDiffRow extends React.Component<
return placeHolder
}
const { height } = rowSelectableGroup
const {
height,
selectionState,
lineNumbers,
lineNumbersIdentifiers,
diffType,
} = rowSelectableGroup
const style = { height }
const onlyOneLine = lineNumbers.length === 1
const hunkHandleClassName = classNames('hunk-handle', 'hoverable', {
// selected is a class if any line in the group is selected
selected: selectionState !== DiffSelectionType.None,
})
const checkAllId = lineNumbersIdentifiers.join('-')
/* The hunk-handle is a a single element with a calculated height of all the
rows in the selectable group (See `getRowSelectableGroupHeight` in
@ -561,28 +595,75 @@ export class SideBySideDiffRow extends React.Component<
hacky side.
*/
const hunkHandle = (
// eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions
<div
className="hunk-handle hoverable"
<label
htmlFor={checkAllId}
onMouseEnter={this.onMouseEnterHunk}
onMouseLeave={this.onMouseLeaveHunk}
onClick={this.onClickHunk}
onContextMenu={this.onContextMenuHunk}
className={hunkHandleClassName}
style={style}
>
{!enableGroupDiffCheckmarks() && (
<div className="increased-hover-surface" style={{ height }} />
)}
</div>
<span className="focus-handle">
{!enableGroupDiffCheckmarks() && (
<div className="increased-hover-surface" style={{ height }} />
)}
{!onlyOneLine && this.getCheckAllOcticon(selectionState)}
{!onlyOneLine && (
<span className="sr-only">
{' '}
Lines {lineNumbers.at(0)} to {lineNumbers.at(-1)}{' '}
{diffType === DiffRowType.Added
? 'added'
: diffType === DiffRowType.Deleted
? 'deleted'
: 'modified'}
</span>
)}
</span>
</label>
)
const checkAllControl = (
<input
className="sr-only"
id={checkAllId}
type="checkbox"
aria-controls={lineNumbersIdentifiers.join(' ')}
aria-checked={
selectionState === DiffSelectionType.All
? true
: selectionState === DiffSelectionType.Partial
? 'mixed'
: false
}
onChange={this.onClickHunk}
/>
)
return (
<>
{placeHolder}
{!onlyOneLine && checkAllControl}
{hunkHandle}
{placeHolder}
</>
)
}
private getCheckAllOcticon = (selectionState: DiffSelectionType) => {
if (!enableGroupDiffCheckmarks()) {
return null
}
if (selectionState === DiffSelectionType.All) {
return <Octicon symbol={diffCheck} />
}
if (selectionState === DiffSelectionType.Partial) {
return <Octicon symbol={octicons.dash} />
}
return null
}
private getLineNumbersContainerID(column: DiffColumn) {
return `line-numbers-${this.props.numRow}-${column}`
}
@ -610,7 +691,12 @@ export class SideBySideDiffRow extends React.Component<
'line-selected': isSelected,
hover: this.props.isHunkHovered,
})
const checkboxId = wrapperID ? wrapperID + '-checkbox' : undefined
// Note: This id is used by the check all aria-controls attribute,
// modification of this should be reflected there.
const checkboxId = `${lineNumbers.filter(ln => ln !== undefined).at(0)}-${
column === DiffColumn.After ? 'after' : 'before'
}`
return (
// eslint-disable-next-line jsx-a11y/no-static-element-interactions
@ -626,7 +712,15 @@ export class SideBySideDiffRow extends React.Component<
<label htmlFor={checkboxId}>
{this.renderLineNumberCheck(isSelected)}
{lineNumbers.map((lineNumber, index) => (
<span key={index}>{lineNumber}</span>
<span key={index}>
{lineNumber && <span className="sr-only">Line </span>}
{lineNumber}
{lineNumber && (
<span className="sr-only">
{column === DiffColumn.After ? ' added' : ' deleted'}
</span>
)}
</span>
))}
</label>
</div>

View file

@ -657,6 +657,8 @@ export class SideBySideDiff extends React.Component<
const { from, to } = range
const { lineNumbers, lineNumbersIdentifiers, diffType } =
this.getRowGroupLineNumberData(row.hunkStartLine)
return {
isFirst: prev === undefined || !selectableType.includes(prev.type),
isLast: next === undefined || !selectableType.includes(next.type),
@ -664,9 +666,63 @@ export class SideBySideDiff extends React.Component<
isFocused: true, // focusedHunk === row.hunkStartLine, - To be added in later PR
selectionState: selection.isRangeSelected(from, to - from + 1),
height: this.getRowSelectableGroupHeight(row.hunkStartLine),
lineNumbers: Array.from(lineNumbers),
lineNumbersIdentifiers,
diffType,
}
}
private getRowGroupLineNumberData = (hunkStartLine: number) => {
const rows = getDiffRows(
this.state.diff,
this.props.showSideBySideDiff,
this.canExpandDiff()
)
const lineNumbers = new Set<number>()
let hasAfter = false
let hasBefore = false
const lineNumbersIdentifiers = rows.flatMap(r => {
if (!('hunkStartLine' in r) || r.hunkStartLine !== hunkStartLine) {
return []
}
if (r.type === DiffRowType.Added) {
lineNumbers.add(r.data.lineNumber)
hasAfter = true
return `${r.data.lineNumber}-after`
}
if (r.type === DiffRowType.Deleted) {
lineNumbers.add(r.data.lineNumber)
hasBefore = true
return `${r.data.lineNumber}-before`
}
if (r.type === DiffRowType.Modified) {
hasAfter = true
hasBefore = true
lineNumbers.add(r.beforeData.lineNumber)
lineNumbers.add(r.afterData.lineNumber)
return [
`${r.beforeData.lineNumber}-before`,
`${r.afterData.lineNumber}-after`,
]
}
return []
})
const diffType =
hasAfter && hasBefore
? DiffRowType.Modified
: hasAfter
? DiffRowType.Added
: DiffRowType.Deleted
return { lineNumbersIdentifiers, lineNumbers, diffType }
}
private getRowSelectableGroupHeight = (hunkStartLine: number) => {
const rows = getDiffRows(
this.state.diff,

View file

@ -31,6 +31,41 @@
flex-direction: row-reverse;
border-right: none;
}
&.context .before {
margin-right: calc(var(--hunk-handle-width) / 2);
}
&.context .after {
margin-right: calc(var(--hunk-handle-width) / 2);
}
&.has-check-all-control.context .before {
margin-right: calc(var(--hunk-handle-width-with-check-all) / 2);
}
&.has-check-all-control.context .after {
margin-left: calc(var(--hunk-handle-width-with-check-all) / 2);
}
&.has-check-all-control {
.hunk-expansion-handle {
padding-left: calc(var(--hunk-handle-width-with-check-all) / 2);
padding-right: calc(var(--hunk-handle-width-with-check-all) / 2);
}
}
.hunk-expansion-handle {
padding-left: calc(var(--hunk-handle-width) / 2);
padding-right: calc(var(--hunk-handle-width) / 2);
}
&.context {
margin-left: var(--hunk-handle-width);
}
&.has-check-all-control.context {
margin-left: var(--hunk-handle-width-with-check-all);
}
}
}
@ -93,21 +128,6 @@
}
}
&.context .before {
margin-right: calc(var(--hunk-handle-width) / 2);
}
&.context .after {
margin-right: calc(var(--hunk-handle-width) / 2);
}
&.has-check-all-control.context .before {
margin-right: calc(var(--hunk-handle-width-with-check-all) / 2);
}
&.has-check-all-control.context .after {
margin-left: calc(var(--hunk-handle-width-with-check-all) / 2);
}
.before,
.after {
width: 50%;
@ -168,6 +188,13 @@
}
}
input:focus-visible + label.hunk-handle {
.focus-handle {
outline: 2px solid #0659d6;
outline-offset: 3px;
}
}
.hunk-handle-place-holder,
.hunk-handle {
width: var(--hunk-handle-width);
@ -177,7 +204,27 @@
.hunk-handle {
position: absolute;
display: flex;
left: calc(50% - var(--hunk-handle-width) / 2);
color: white;
padding-top: 3px;
text-align: center;
.focus-handle {
border-radius: var(--border-radius);
height: calc(100% + 4px);
width: 100%;
margin-top: -4px;
padding-top: 3px;
}
.octicon {
height: 12px;
}
&.selected {
background-color: var(--diff-selected-border-color);
}
}
&.has-check-all-control {
@ -189,6 +236,13 @@
.hunk-handle {
left: calc(50% - var(--hunk-handle-width-with-check-all) / 2);
}
input:focus-visible + label.hunk-handle {
.focus-handle {
outline: 2px solid white;
outline-offset: -3px;
}
}
}
.hunk-handle {
@ -214,18 +268,6 @@
}
}
&:has(.line-number.selectable.line-selected) {
.hunk-handle {
background-color: var(--diff-selected-border-color);
}
}
&:has(.line-number.selectable) {
.hunk-handle {
background-color: var(--diff-empty-hunk-handle);
}
}
.hunk-expansion-handle {
button {
overflow: inherit;
@ -404,18 +446,6 @@
left: 0;
}
&.has-check-all-control {
.hunk-expansion-handle {
padding-left: calc(var(--hunk-handle-width-with-check-all) / 2);
padding-right: calc(var(--hunk-handle-width-with-check-all) / 2);
}
}
.hunk-expansion-handle {
padding-left: calc(var(--hunk-handle-width) / 2);
padding-right: calc(var(--hunk-handle-width) / 2);
}
&.context .before {
margin-right: 0;
}
@ -423,14 +453,6 @@
margin-left: 0;
}
&.context {
margin-left: var(--hunk-handle-width);
}
&.has-check-all-control.context {
margin-left: var(--hunk-handle-width-with-check-all);
}
.line-number {
width: calc(var(--width-line-number) * 2);
border-left: none;