Skip to content

Commit

Permalink
select: patch VSCodeCheckbox to fix onChange
Browse files Browse the repository at this point in the history
Summary:
Add a patched version of `VSCodeCheckbox` that ignores `onChange` if `checked`
matches the last render. This allows proper support of "managed" checked
toggle, for example, (de-)selecting all lines will (de-)select the chunk
checkbox.

By using the `onChange` event directly we got better support for keyboard
events.

Reviewed By: evangrayk

Differential Revision: D48520304

fbshipit-source-id: 3b42569c7168779a75daba066e0a88cca4fff9ed
  • Loading branch information
quark-zju authored and facebook-github-bot committed Aug 23, 2023
1 parent 3e9c5cf commit 3c7b4f5
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 12 deletions.
23 changes: 11 additions & 12 deletions addons/isl/src/PartialFileSelection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import type {RangeInfo} from './TextEditable';
import type {ChunkSelectState, LineRegion, SelectLine} from './stackEdit/chunkSelectState';

import {TextEditable} from './TextEditable';
import {VSCodeCheckbox} from './VSCodeCheckbox';
import {T, t} from './i18n';
import {VSCodeCheckbox, VSCodeRadio} from '@vscode/webview-ui-toolkit/react';
import {VSCodeRadio} from '@vscode/webview-ui-toolkit/react';
import {Set as ImSet} from 'immutable';
import {useRef, useState} from 'react';
import {notEmpty} from 'shared/utils';
Expand Down Expand Up @@ -135,18 +136,16 @@ function PartialFileSelectionWithCheckbox(props: Props & {unified?: boolean}) {
const selectedCount = region.lines.reduce((acc, line) => acc + (line.selected ? 1 : 0), 0);
const indeterminate = selectedCount > 0 && selectedCount < selectableCount;
const checked = selectedCount === selectableCount;
// Note: VSCodeCheckbox's onClick or onChange are not really React events
// and are hard to get right (ex. onChange can be triggered by re-rendering
// with a different `checked` state without events on the checkbox itself).
// So we use onClick on the parent element.
// FIXME: This does not work for keyboard checkbox events.
lineCheckbox.push(
<div className="checkbox-anchor">
<div
key={`${key}c`}
className="checkbox-container"
onClick={_e => toogleLineOrRegion(region.lines[0], region)}>
<VSCodeCheckbox checked={checked} indeterminate={indeterminate} />
<div className="checkbox-anchor" key={`${key}c`}>
<div className="checkbox-container">
<VSCodeCheckbox
checked={checked}
indeterminate={indeterminate}
onChange={() => {
toogleLineOrRegion(region.lines[0], region);
}}
/>
</div>
</div>,
);
Expand Down
48 changes: 48 additions & 0 deletions addons/isl/src/VSCodeCheckbox.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import type {Constructable} from '@microsoft/fast-element';

import * as OriginalToolkit from '@vscode/webview-ui-toolkit/react';
import {useRef} from 'react';

/**
* A patched version of VSCodeCheckbox. Do not trigger `onChange` if `checked`
* is managed and changed by other places.
*
* See https://github.com/microsoft/vscode-webview-ui-toolkit/issues/408.
*/
export function VSCodeCheckbox(props: VSCodeCheckboxProps) {
// props.checked from the last render.
const ignoreChecked = useRef<boolean | null>(null);
if (ignoreChecked.current !== props.checked && props.checked != null) {
ignoreChecked.current = props.checked;
}

// Replace onChange handler to ignore managed 'checked' changed by re-render.
const onChange: VSCodeCheckboxProps['onChange'] =
props.onChange == null
? undefined
: (...args) => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const checked = (args[0].target as any).checked;
if (ignoreChecked.current == checked) {
return;
}
// This seems obviously correct yet tsc cannot type check it.
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
return props.onChange(...args);
};

return <OriginalToolkit.VSCodeCheckbox {...props} onChange={onChange} />;
}

type ExtractProps<T> = T extends Constructable<React.Component<infer P, unknown, unknown>>
? P
: never;
type VSCodeCheckboxProps = ExtractProps<typeof OriginalToolkit.VSCodeCheckbox>;

0 comments on commit 3c7b4f5

Please sign in to comment.