Shortcuts sorting needs a test

[krassowski]

A test for sorting of shortcuts in the shortcuts table UI:

could be added in:

jupyterlab/packages/shortcuts-extension/test/components/ShortcutUI.spec.ts

Line 47 in 5af2cc9

describe('ShortcutUI', () => {

it would be good to test sorting by each of the sortable columns (category, column, source, selectors).

[krassowski]

The test would be for sortShortcuts function, after :

jupyterlab/packages/shortcuts-extension/src/components/ShortcutUI.tsx

Lines 423 to 453 in 5af2cc9

	/**
	* Sort shortcut list using current sort property.
	*/
	sortShortcuts(): void {
	const shortcuts: IShortcutTarget[] = this.state.filteredShortcutList;
	let sortCriteria = this.state.currentSort;
	if (sortCriteria === 'command') {
	sortCriteria = 'label';
	}
	const getValue = (target: IShortcutTarget): string => {
	if (sortCriteria === 'source') {
	return target.keybindings.every(k => k.isDefault) ? 'default' : 'other';
	}
	return target[sortCriteria] ?? '';
	};
	shortcuts.sort((a: IShortcutTarget, b: IShortcutTarget) => {
	const compareA: string = getValue(a);
	const compareB: string = getValue(b);
	if (compareA < compareB) {
	return -1;
	} else if (compareA > compareB) {
	return 1;
	} else {
	const aLabel = a['label'] ?? '';
	const bLabel = b['label'] ?? '';
	return aLabel < bLabel ? -1 : aLabel > bLabel ? 1 : 0;
	}
	});
	this.setState({ filteredShortcutList: shortcuts });
	}

and should verify that filteredShortcutList is in expected order. It will need to call the updateSort method to configure by which column the table should be sorted.

jupyterlab/packages/shortcuts-extension/src/components/ShortcutUI.tsx

Lines 414 to 421 in 5af2cc9

	/**
	* Set the sort order for the shortcuts listing.
	*/
	updateSort = (value: IShortcutUI.ColumnId): void => {
	if (value !== this.state.currentSort) {
	this.setState({ currentSort: value }, this.sortShortcuts);
	}
	};

[milinm]

Hello, I am a student looking to contribute to my first open source project. May I have this issue assigned to me?

[krassowski]

Closing as solved by #16098.

@milinm if you would like to work on other good first issues feel welcome to just open a draft PR, or comment with your plan and self-assign (you can assign yourself after leaving a comment).