Merge pull request #3489 from balena-io/direct-select-drive

patch: Select drive on list interaction rather than modal closing

CODEOWNERS

@@ -1,2 +0,0 @@
-* @thundron @zvin @jviotti
-/scripts @nazrhom

Makefile

@@ -86,6 +86,7 @@ TARGET_ARCH ?= $(HOST_ARCH)
 # Electron
 # ---------------------------------------------------------------------
 electron-develop:
+	git submodule update --init && \
 	$(RESIN_SCRIPTS)/electron/install.sh \
 		-b $(shell pwd) \
 		-r $(TARGET_ARCH) \

lib/gui/app/components/drive-selector/drive-selector.tsx

@@ -43,6 +43,7 @@ import {
 } from '../../styled-components';
 
 import { SourceMetadata } from '../source-selector/source-selector';
+import { middleEllipsis } from '../../utils/middle-ellipsis';
 
 interface UsbbootDrive extends sourceDestination.UsbbootDrive {
 	progress: number;
@@ -136,17 +137,18 @@ const InitProgress = styled(
 `;
 
 export interface DriveSelectorProps
-	extends Omit<ModalProps, 'done' | 'cancel'> {
+	extends Omit<ModalProps, 'done' | 'cancel' | 'onSelect'> {
 	write: boolean;
 	multipleSelection: boolean;
 	showWarnings?: boolean;
-	cancel: () => void;
+	cancel: (drives: DrivelistDrive[]) => void;
 	done: (drives: DrivelistDrive[]) => void;
 	titleLabel: string;
 	emptyListLabel: string;
 	emptyListIcon: JSX.Element;
 	selectedList?: DrivelistDrive[];
 	updateSelectedList?: () => DrivelistDrive[];
+	onSelect?: (drive: DrivelistDrive) => void;
 }
 
 interface DriveSelectorState {
@@ -167,12 +169,14 @@ export class DriveSelector extends React.Component<
 > {
 	private unsubscribe: (() => void) | undefined;
 	tableColumns: Array<TableColumn<Drive>>;
+	originalList: DrivelistDrive[];
 
 	constructor(props: DriveSelectorProps) {
 		super(props);
 
 		const defaultMissingDriversModalState: { drive?: DriverlessDrive } = {};
 		const selectedList = this.props.selectedList || [];
+		this.originalList = [...(this.props.selectedList || [])];
 
 		this.state = {
 			drives: getDrives(),
@@ -199,7 +203,9 @@ export class DriveSelector extends React.Component<
 										fill={drive.isSystem ? '#fca321' : '#8f9297'}
 									/>
 								)}
-								<Txt ml={(hasWarnings && 8) || 0}>{description}</Txt>
+								<Txt ml={(hasWarnings && 8) || 0}>
+									{middleEllipsis(description, 32)}
+								</Txt>
 							</Flex>
 						);
 					}
@@ -259,7 +265,7 @@ export class DriveSelector extends React.Component<
 		return (
 			isUsbbootDrive(drive) ||
 			isDriverlessDrive(drive) ||
-			!isDriveValid(drive, image) ||
+			!isDriveValid(drive, image, this.props.write) ||
 			(this.props.write && drive.isReadOnly)
 		);
 	}
@@ -348,16 +354,6 @@ export class DriveSelector extends React.Component<
 		}
 	}
 
-	private deselectingAll(rows: DrivelistDrive[]) {
-		return (
-			rows.length > 0 &&
-			rows.length === this.state.selectedList.length &&
-			this.state.selectedList.every(
-				(d) => rows.findIndex((r) => d.device === r.device) > -1,
-			)
-		);
-	}
-
 	componentDidMount() {
 		this.unsubscribe = store.subscribe(() => {
 			const drives = getDrives();
@@ -408,7 +404,7 @@ export class DriveSelector extends React.Component<
 					</Flex>
 				}
 				titleDetails={<Txt fontSize={11}>{getDrives().length} found</Txt>}
-				cancel={cancel}
+				cancel={() => cancel(this.originalList)}
 				done={() => done(selectedList)}
 				action={`Select (${selectedList.length})`}
 				primaryButtonProps={{
@@ -448,14 +444,34 @@ export class DriveSelector extends React.Component<
 							onCheck={(rows: Drive[]) => {
 								let newSelection = rows.filter(isDrivelistDrive);
 								if (this.props.multipleSelection) {
-									if (this.deselectingAll(newSelection)) {
+									if (rows.length === 0) {
 										newSelection = [];
 									}
+									const deselecting = selectedList.filter(
+										(selected) =>
+											newSelection.filter(
+												(row) => row.device === selected.device,
+											).length === 0,
+									);
+									const selecting = newSelection.filter(
+										(row) =>
+											selectedList.filter(
+												(selected) => row.device === selected.device,
+											).length === 0,
+									);
+									deselecting.concat(selecting).forEach((row) => {
+										if (this.props.onSelect) {
+											this.props.onSelect(row);
+										}
+									});
 									this.setState({
 										selectedList: newSelection,
 									});
 									return;
 								}
+								if (this.props.onSelect) {
+									this.props.onSelect(newSelection[newSelection.length - 1]);
+								}
 								this.setState({
 									selectedList: newSelection.slice(newSelection.length - 1),
 								});
@@ -467,6 +483,9 @@ export class DriveSelector extends React.Component<
 								) {
 									return;
 								}
+								if (this.props.onSelect) {
+									this.props.onSelect(row);
+								}
 								const index = selectedList.findIndex(
 									(d) => d.device === row.device,
 								);

lib/gui/app/components/source-selector/source-selector.tsx

@@ -558,6 +558,12 @@ export class SourceSelector extends React.Component<
 		this.setState({ defaultFlowActive });
 	}
 
+	private closeModal() {
+		this.setState({
+			showDriveSelector: false,
+		});
+	}
+
 	// TODO add a visual change when dragging a file over the selector
 	public render() {
 		const { flashing } = this.props;
@@ -661,6 +667,9 @@ export class SourceSelector extends React.Component<
 
 				{this.state.warning != null && (
 					<SmallModal
+						style={{
+							boxShadow: '0 3px 7px rgba(0, 0, 0, 0.3)',
+						}}
 						titleElement={
 							<span>
 								<ExclamationTriangleSvg fill="#fca321" height="1em" />{' '}
@@ -736,21 +745,30 @@ export class SourceSelector extends React.Component<
 						titleLabel="Select source"
 						emptyListLabel="Plug a source drive"
 						emptyListIcon={<SrcSvg width="40px" />}
-						cancel={() => {
+						cancel={(originalList) => {
-							this.setState({
+							if (originalList.length) {
-								showDriveSelector: false,
+								const originalSource = originalList[0];
-							});
+								if (selectionImage?.drive?.device !== originalSource.device) {
-						}}
+									this.selectSource(
-						done={async (drives: DrivelistDrive[]) => {
+										originalSource,
-							if (drives.length) {
-								await this.selectSource(
-									drives[0],
 										sourceDestination.BlockDevice,
 									);
 								}
-							this.setState({
+							} else {
-								showDriveSelector: false,
+								selectionState.deselectImage();
-							});
+							}
+							this.closeModal();
+						}}
+						done={() => this.closeModal()}
+						onSelect={(drive) => {
+							if (drive) {
+								if (
+									selectionState.getImage()?.drive?.device === drive?.device
+								) {
+									return selectionState.deselectImage();
+								}
+								this.selectSource(drive, sourceDestination.BlockDevice);
+							}
 						}}
 					/>
 				)}

lib/gui/app/components/target-selector/target-selector.tsx

@@ -28,6 +28,7 @@ import {
 	getSelectedDrives,
 	deselectDrive,
 	selectDrive,
+	deselectAllDrives,
 } from '../../models/selection-state';
 import { observe } from '../../models/store';
 import * as analytics from '../../modules/analytics';
@@ -164,11 +165,30 @@ export const TargetSelector = ({
 			{showTargetSelectorModal && (
 				<TargetSelectorModal
 					write={true}
-					cancel={() => setShowTargetSelectorModal(false)}
+					cancel={(originalList) => {
-					done={(modalTargets) => {
+						if (originalList.length) {
-						selectAllTargets(modalTargets);
+							selectAllTargets(originalList);
+						} else {
+							deselectAllDrives();
+						}
 						setShowTargetSelectorModal(false);
 					}}
+					done={(modalTargets) => {
+						if (modalTargets.length === 0) {
+							deselectAllDrives();
+						}
+						setShowTargetSelectorModal(false);
+					}}
+					onSelect={(drive) => {
+						if (
+							getSelectedDrives().find(
+								(selectedDrive) => selectedDrive.device === drive.device,
+							)
+						) {
+							return deselectDrive(drive.device);
+						}
+						selectDrive(drive.device);
+					}}
 				/>
 			)}
 		</Flex>

lib/gui/app/models/leds.ts

@@ -17,12 +17,11 @@
 import * as _ from 'lodash';
 import { Animator, AnimationFunction, Color, RGBLed } from 'sys-class-rgb-led';
 
-import {
+import { DrivelistDrive } from '../../../shared/drive-constraints';
-	isSourceDrive,
+import { getDrives } from './available-drives';
-	DrivelistDrive,
+import { getImage, getSelectedDrives } from './selection-state';
-} from '../../../shared/drive-constraints';
 import * as settings from './settings';
-import { DEFAULT_STATE, observe } from './store';
+import { observe, store } from './store';
 
 const leds: Map<string, RGBLed> = new Map();
 const animator = new Animator([], 10);
@@ -40,7 +39,7 @@ function createAnimationFunction(
 ): AnimationFunction {
 	return (t: number): Color => {
 		const intensity = intensityFunction(t);
-		return color.map((v) => v * intensity) as Color;
+		return color.map((v: number) => v * intensity) as Color;
 	};
 }
 
@@ -160,35 +159,28 @@ export function updateLeds({
 	animator.mapping = mapping;
 }
 
-interface DeviceFromState {
-	devicePath?: string;
-	device: string;
-}
-
 let ledsState: LedsState | undefined;
 
-function stateObserver(state: typeof DEFAULT_STATE) {
+function stateObserver() {
-	const s = state.toJS();
+	const s = store.getState().toJS();
 	let step: 'main' | 'flashing' | 'verifying' | 'finish';
 	if (s.isFlashing) {
 		step = s.flashState.type;
 	} else {
 		step = s.lastAverageFlashingSpeed == null ? 'main' : 'finish';
 	}
-	const availableDrives = s.availableDrives.filter(
+	const availableDrives = getDrives().filter(
-		(d: DeviceFromState) => d.devicePath,
+		(d: DrivelistDrive) => d.devicePath,
 	);
-	const sourceDrivePath = availableDrives.filter((d: DrivelistDrive) =>
+	const sourceDrivePath = getImage()?.drive?.devicePath;
-		isSourceDrive(d, s.selection.image),
-	)[0]?.devicePath;
 	const availableDrivesPaths = availableDrives.map(
-		(d: DeviceFromState) => d.devicePath,
+		(d: DrivelistDrive) => d.devicePath,
 	);
 	let selectedDrivesPaths: string[];
 	if (step === 'main') {
-		selectedDrivesPaths = availableDrives
+		selectedDrivesPaths = getSelectedDrives()
-			.filter((d: DrivelistDrive) => s.selection.devices.includes(d.device))
+			.filter((drive) => drive.devicePath !== null)
-			.map((d: DrivelistDrive) => d.devicePath);
+			.map((drive) => drive.devicePath) as string[];
 	} else {
 		selectedDrivesPaths = s.devicePaths;
 	}
@@ -201,7 +193,7 @@ function stateObserver(state: typeof DEFAULT_STATE) {
 		availableDrives: availableDrivesPaths,
 		selectedDrives: selectedDrivesPaths,
 		failedDrives: failedDevicePaths,
-	};
+	} as LedsState;
 	if (!_.isEqual(newLedsState, ledsState)) {
 		updateLeds(newLedsState);
 		ledsState = newLedsState;

lib/gui/app/pages/main/MainPage.tsx

@@ -276,7 +276,7 @@ export class MainPage extends React.Component<
 					style={{
 						// Allow window to be dragged from header
 						// @ts-ignore
-						'-webkit-app-region': 'drag',
+						WebkitAppRegion: 'drag',
 						position: 'relative',
 						zIndex: 2,
 					}}
@@ -304,7 +304,7 @@ export class MainPage extends React.Component<
 							onClick={() => this.setState({ hideSettings: false })}
 							style={{
 								// Make touch events click instead of dragging
-								'-webkit-app-region': 'no-drag',
+								WebkitAppRegion: 'no-drag',
 							}}
 						/>
 						{!settings.getSync('disableExternalLinks') && (
@@ -319,7 +319,7 @@ export class MainPage extends React.Component<
 								tabIndex={6}
 								style={{
 									// Make touch events click instead of dragging
-									'-webkit-app-region': 'no-drag',
+									WebkitAppRegion: 'no-drag',
 								}}
 							/>
 						)}

lib/shared/drive-constraints.ts

@@ -104,24 +104,19 @@ export function isDriveLargeEnough(
 	return driveSize >= (image.size || UNKNOWN_SIZE);
 }
 
-/**
- * @summary Check if a drive is disabled (i.e. not ready for selection)
- */
-export function isDriveDisabled(drive: DrivelistDrive): boolean {
-	return drive.disabled || false;
-}
-
 /**
  * @summary Check if a drive is valid, i.e. large enough for an image
  */
 export function isDriveValid(
 	drive: DrivelistDrive,
 	image?: SourceMetadata,
+	write: boolean = true,
 ): boolean {
 	return (
+		!write ||
+		(!drive.disabled &&
 			isDriveLargeEnough(drive, image) &&
-		!isSourceDrive(drive, image as SourceMetadata) &&
+			!isSourceDrive(drive, image as SourceMetadata))
-		!isDriveDisabled(drive)
 	);
 }
 

npm-shrinkwrap.json

@@ -3535,11 +3535,11 @@
       "version": "5.7.1",
       "resolved": "https://registry.npmjs.org/buffer/-/buffer-5.7.1.tgz",
       "integrity": "sha512-EHcyIPBQ4BSGlvjB16k5KgAJ27CIsHY/2JBmCRReo48y9rQ3MaUzWX3KVlBa4U7MyX02HdVj0K7C3WaB3ju7FQ==",
+      "dev": true,
       "requires": {
         "base64-js": "^1.3.1",
         "ieee754": "^1.1.13"
-      },
+      }
-      "dev": true
     },
     "buffer-alloc": {
       "version": "1.2.0",

tests/shared/drive-constraints.spec.ts

@@ -514,40 +514,6 @@ describe('Shared: DriveConstraints', function () {
 		});
 	});
 
-	describe('.isDriveDisabled()', function () {
-		it('should return true if the drive is disabled', function () {
-			const result = constraints.isDriveDisabled(({
-				device: '/dev/disk1',
-				size: 1000000000,
-				isReadOnly: false,
-				disabled: true,
-			} as unknown) as constraints.DrivelistDrive);
-
-			expect(result).to.be.true;
-		});
-
-		it('should return false if the drive is not disabled', function () {
-			const result = constraints.isDriveDisabled(({
-				device: '/dev/disk1',
-				size: 1000000000,
-				isReadOnly: false,
-				disabled: false,
-			} as unknown) as constraints.DrivelistDrive);
-
-			expect(result).to.be.false;
-		});
-
-		it('should return false if "disabled" is undefined', function () {
-			const result = constraints.isDriveDisabled({
-				device: '/dev/disk1',
-				size: 1000000000,
-				isReadOnly: false,
-			} as constraints.DrivelistDrive);
-
-			expect(result).to.be.false;
-		});
-	});
-
 	describe('.isDriveSizeRecommended()', function () {
 		const image: SourceMetadata = {
 			description: 'rpi.img',

webpack.config.ts

@@ -354,6 +354,7 @@ const guiConfig = {
 	entry: {
 		gui: path.join(__dirname, 'lib', 'gui', 'app', 'renderer.ts'),
 	},
+	// entry: path.join(__dirname, 'lib', 'gui', 'app', 'renderer.ts'),
 	plugins: [
 		...commonConfig.plugins,
 		new CopyPlugin({

webpack.dev.config.ts

@@ -10,6 +10,8 @@ const [
 
 configs.forEach((config) => {
 	config.mode = 'development';
+	// @ts-ignore
+	config.devtool = 'source-map';
 });
 
 guiConfig.devServer = {