Fix key combinations being prevented in dbSafe v-input when it's a leading number (#16668)

* Fix key combinations being prevented in v-input

* should normalize accented characters first

* add tests for processValue and emitValue

* export keyMap from use-shortcut to keep things DRY

* try to add test for use-shortcut composable

* move systemKeys to use-shortcut

* add "capslock" & "enter" to systemKeys

app/src/components/v-input.test.ts

@@ -1,9 +1,9 @@
-import { test, expect } from 'vitest';
+import { Focus } from '@/__utils__/focus';
 import { mount } from '@vue/test-utils';
+import { GlobalMountOptions } from '@vue/test-utils/dist/types';
+import { describe, expect, test } from 'vitest';
 
 import VInput from './v-input.vue';
-import { GlobalMountOptions } from '@vue/test-utils/dist/types';
-import { Focus } from '@/__utils__/focus';
 
 const global: GlobalMountOptions = {
 	stubs: ['v-icon'],
@@ -64,3 +64,173 @@ test('modelValue dbSafe', async () => {
 
 	expect(wrapper.emitted()['update:modelValue'][0]).toEqual(['this_hould_be_D_save']);
 });
+
+describe('processValue', () => {
+	const commonTestScenarios = [
+		{
+			scenario: 'should allow slug safe characters',
+			event: { key: 'a' },
+			shouldDefaultPrevented: false,
+		},
+		{
+			scenario: 'should not allow non slug safe characters',
+			event: { key: '$' },
+			shouldDefaultPrevented: true,
+		},
+		{
+			scenario: 'should allow system keys',
+			event: { key: 'Control' }, // also tests whether "Control" is mapped to "meta"
+			shouldDefaultPrevented: false,
+		},
+		{
+			scenario: 'should allow arrow keys',
+			event: { key: 'ArrowUp' },
+			shouldDefaultPrevented: false,
+		},
+	];
+
+	test.each([
+		...commonTestScenarios,
+		{
+			scenario: 'should not allow trailing space after the slug separator',
+			event: { key: ' ' },
+			shouldDefaultPrevented: true,
+		},
+	])('slug input %scenario', async ({ event, shouldDefaultPrevented }) => {
+		const wrapper = mount(VInput, {
+			props: {
+				// default slug separator to test the "should not allow trailing space after slug separator" scenario
+				modelValue: '-',
+				slug: true,
+			},
+			global,
+		});
+
+		const inputElement = wrapper.find('input').element;
+		// mock keyboard event
+		const keyboardEvent = new KeyboardEvent('keydown', event);
+		// manually attach the input element as the mocked event's target
+		Object.defineProperty(keyboardEvent, 'target', { value: inputElement });
+
+		wrapper.vm.processValue(keyboardEvent);
+
+		expect(keyboardEvent.defaultPrevented).toBe(shouldDefaultPrevented);
+	});
+
+	test.each([
+		...commonTestScenarios,
+		{
+			scenario: 'should allow system key combinations with number when entering the first character',
+			event: { key: '1', shiftKey: true },
+			shouldDefaultPrevented: false,
+		},
+		{
+			scenario: 'should not allow number when entering the first character',
+			event: { key: '1' },
+			shouldDefaultPrevented: true,
+		},
+	])('dbSafe input %scenario', async ({ event, shouldDefaultPrevented }) => {
+		const wrapper = mount(VInput, {
+			props: {
+				dbSafe: true,
+			},
+			global,
+		});
+
+		const inputElement = wrapper.find('input').element;
+		// mock keyboard event
+		const keyboardEvent = new KeyboardEvent('keydown', event);
+		// manually attach the input element as the mocked event's target
+		Object.defineProperty(keyboardEvent, 'target', { value: inputElement });
+
+		wrapper.vm.processValue(keyboardEvent);
+
+		expect(keyboardEvent.defaultPrevented).toBe(shouldDefaultPrevented);
+	});
+});
+
+describe('emitValue', () => {
+	test('should emit null value when empty', async () => {
+		const wrapper = mount(VInput, {
+			props: {
+				modelValue: '',
+				nullable: true,
+			},
+			global,
+		});
+
+		await wrapper.find('input').trigger('input');
+
+		expect(wrapper.emitted()['update:modelValue'][0]).toEqual([null]);
+	});
+
+	test('should emit number when type is number', async () => {
+		const wrapper = mount(VInput, {
+			props: {
+				type: 'number',
+				modelValue: '1',
+			},
+			global,
+		});
+
+		await wrapper.find('input').trigger('input');
+
+		expect(wrapper.emitted()['update:modelValue'][0]).toEqual([1]);
+	});
+
+	test('should turn ending space into slug separator for slug input', async () => {
+		const wrapper = mount(VInput, {
+			props: {
+				modelValue: 'test ',
+				slug: true,
+			},
+			global,
+		});
+
+		await wrapper.find('input').trigger('input');
+
+		expect(wrapper.emitted()['update:modelValue'][0]).toEqual(['test-']);
+	});
+
+	test('should turn space into underscores for dbSafe input', async () => {
+		const wrapper = mount(VInput, {
+			props: {
+				modelValue: 'a custom field',
+				dbSafe: true,
+			},
+			global,
+		});
+
+		await wrapper.find('input').trigger('input');
+
+		expect(wrapper.emitted()['update:modelValue'][0]).toEqual(['a_custom_field']);
+	});
+
+	test('should prevent pasting of non db safe characters for dbSafe input', async () => {
+		const wrapper = mount(VInput, {
+			props: {
+				modelValue: '$test_field',
+				dbSafe: true,
+			},
+			global,
+		});
+
+		await wrapper.find('input').trigger('input');
+
+		expect(wrapper.emitted()['update:modelValue'][0]).toEqual(['test_field']);
+	});
+
+	test('should normalize accented characters for dbSafe input', async () => {
+		const wrapper = mount(VInput, {
+			props: {
+				modelValue: 'à_test_field',
+				dbSafe: true,
+			},
+			global,
+		});
+
+		await wrapper.find('input').trigger('input');
+
+		expect(wrapper.emitted()['update:modelValue'][0]).toEqual(['a_test_field']);
+	});
+});

app/src/components/v-input.vue

@@ -62,9 +62,10 @@ export default {
 </script>
 
 <script lang="ts" setup>
-import { computed, ref, useAttrs } from 'vue';
-import { omit } from 'lodash';
+import { keyMap, systemKeys } from '@/composables/use-shortcut';
 import slugify from '@sindresorhus/slugify';
+import { omit } from 'lodash';
+import { computed, ref, useAttrs } from 'vue';
 
 interface Props {
 	/** Autofocusses the input on render */
@@ -172,8 +173,7 @@ const isStepDownAllowed = computed(() => {
 
 function processValue(event: KeyboardEvent) {
 	if (!event.key) return;
-	const key = event.key.toLowerCase();
-	const systemKeys = ['meta', 'shift', 'alt', 'backspace', 'delete', 'tab'];
+	const key = event.key in keyMap ? keyMap[event.key] : event.key.toLowerCase();
 	const value = (event.target as HTMLInputElement).value;
 
 	if (props.slug === true) {
@@ -199,8 +199,10 @@ function processValue(event: KeyboardEvent) {
 			event.preventDefault();
 		}
 
+		const isCombinationWithSystemKeys = event.metaKey || event.ctrlKey || event.shiftKey || event.altKey;
+
 		// Prevent leading number
-		if (value.length === 0 && '0123456789'.split('').includes(key)) {
+		if (value.length === 0 && '0123456789'.split('').includes(key) && !isCombinationWithSystemKeys) {
 			event.preventDefault();
 		}
 	}
@@ -238,10 +240,10 @@ function emitValue(event: InputEvent) {
 
 		if (props.dbSafe === true) {
 			value = value.replace(/\s/g, '_');
-			// prevent pasting of non dbSafeCharacters from bypassing the keydown checks
-			value = value.replace(/[^a-zA-Z0-9_]/g, '');
 			// Replace é -> e etc
 			value = value.normalize('NFD').replace(/[\u0300-\u036f]/g, '');
+			// prevent pasting of non dbSafeCharacters from bypassing the keydown checks
+			value = value.replace(/[^a-zA-Z0-9_]/g, '');
 		}
 
 		emit('update:modelValue', value);

app/src/composables/use-shortcut.test.ts

@@ -0,0 +1,52 @@
+import { mount } from '@vue/test-utils';
+import { afterEach, beforeEach, describe, expect, Mock, test, vi } from 'vitest';
+import { defineComponent, h } from 'vue';
+
+import { useShortcut } from './use-shortcut';
+
+function getTestComponent(shortcut: string, handler: () => void) {
+	return defineComponent({
+		setup() {
+			useShortcut(shortcut, handler);
+		},
+		render: () => h('div'),
+	});
+}
+
+describe('useShortcut', () => {
+	let shortcutHandler: Mock;
+
+	beforeEach(() => {
+		shortcutHandler = vi.fn();
+	});
+
+	afterEach(() => {
+		vi.clearAllMocks();
+	});
+
+	test.each(['Control', 'Command'])('should map "%s" key to "meta"', async (testKey) => {
+		const keys = ['meta', 's'];
+		const testComponent = getTestComponent(keys.join('+'), shortcutHandler);
+		const wrapper = mount(testComponent, { attachTo: document.body });
+
+		// intentionally not using keys[0] as we want to test Control/Command, not meta itself
+		await wrapper.trigger('keydown', { key: testKey });
+		await wrapper.trigger('keydown', { key: keys[1] });
+		await wrapper.trigger('keyup', { key: testKey });
+
+		expect(shortcutHandler).toHaveBeenCalledOnce();
+	});
+
+	test('should trigger with combination of shortcut keys', async () => {
+		const keys = ['meta', 'alt', 'c'];
+		const testComponent = getTestComponent(keys.join('+'), shortcutHandler);
+		const wrapper = mount(testComponent, { attachTo: document.body });
+
+		for (const key of keys) {
+			await wrapper.trigger('keydown', { key });
+		}
+		await wrapper.trigger('keyup', { key: keys[0] });
+
+		expect(shortcutHandler).toHaveBeenCalledOnce();
+	});
+});

app/src/composables/use-shortcut.ts

@@ -2,6 +2,13 @@ import { ComponentPublicInstance, onMounted, onUnmounted, Ref, ref } from 'vue';
 
 type ShortcutHandler = (event: KeyboardEvent, cancelNext: () => void) => void | any | boolean;
 
+export const keyMap: Record<string, string> = {
+	Control: 'meta',
+	Command: 'meta',
+};
+
+export const systemKeys = ['meta', 'shift', 'alt', 'backspace', 'delete', 'tab', 'capslock', 'enter'];
+
 const keysdown: Set<string> = new Set([]);
 const handlers: Record<string, ShortcutHandler[]> = {};
 
@@ -62,15 +69,11 @@ export function useShortcut(
 }
 
 function mapKeys(key: KeyboardEvent) {
-	const map: Record<string, string> = {
-		Control: 'meta',
-		Command: 'meta',
-	};
 	const isLatinAlphabet = /^[a-zA-Z0-9]*?$/g;
 
 	let keyString = key.key.match(isLatinAlphabet) === null ? key.code.replace(/(Key|Digit)/g, '') : key.key;
 
-	keyString = keyString in map ? map[keyString] : keyString;
+	keyString = keyString in keyMap ? keyMap[keyString] : keyString;
 	keyString = keyString.toLowerCase();
 
 	return keyString;