From 04008347b044b0a0af48732177d0c94b304d4b5f Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 13 Nov 2024 10:44:53 -0800 Subject: [PATCH 1/4] Move string util for name syntax to a helper function --- app/components/form/fields/NameField.tsx | 9 ++---- app/ui/lib/Combobox.tsx | 16 ++++++++-- app/util/str.spec.tsx | 38 ++++++++++++++++++++++++ app/util/str.ts | 20 +++++++++++++ 4 files changed, 73 insertions(+), 10 deletions(-) diff --git a/app/components/form/fields/NameField.tsx b/app/components/form/fields/NameField.tsx index 7f3100c978..f7d7ae7d79 100644 --- a/app/components/form/fields/NameField.tsx +++ b/app/components/form/fields/NameField.tsx @@ -7,7 +7,7 @@ */ import type { FieldPath, FieldValues } from 'react-hook-form' -import { capitalize } from '~/util/str' +import { capitalize, nameSyntax } from '~/util/str' import { TextField, type TextFieldProps } from './TextField' @@ -30,12 +30,7 @@ export function NameField< required={required} label={label} name={name} - transform={(value) => - value - .toLowerCase() - .replace(/[\s_]+/g, '-') - .replace(/[^a-z0-9-]/g, '') - } + transform={(value) => nameSyntax(value)} // https://www.stefanjudis.com/snippets/turn-off-password-managers/ data-1p-ignore data-bwignore diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx index 05523b7054..fadc31c5f7 100644 --- a/app/ui/lib/Combobox.tsx +++ b/app/ui/lib/Combobox.tsx @@ -19,6 +19,8 @@ import { useEffect, useId, useState, type ReactNode, type Ref } from 'react' import { SelectArrows6Icon } from '@oxide/design-system/icons/react' +import { nameSyntax } from '~/util/str' + import { FieldLabel } from './FieldLabel' import { usePopoverZIndex } from './SideModal' import { TextInputHint } from './TextInput' @@ -119,6 +121,9 @@ export const Combobox = ({ if (allowArbitraryValues && !selectedItemValue) { setQuery('') } + if (selectedItemValue) { + setQuery(selectedItemValue) + } }, [allowArbitraryValues, selectedItemValue]) // If the user has typed in a value that isn't in the list, @@ -193,13 +198,18 @@ export const Combobox = ({ // selectedItemValue is displayed when the user can type in a new value. // Otherwise, use the provided selectedItemLabel displayValue={() => - allowArbitraryValues ? selectedItemValue : selectedItemLabel + selectedItemValue + ? allowArbitraryValues + ? selectedItemValue + : selectedItemLabel + : query } onChange={(event) => { + const value = nameSyntax(event.target.value, false) // updates the query state as the user types, in order to filter the list of items - setQuery(event.target.value) + setQuery(value) // if the parent component wants to know about input changes, call the callback - onInputChange?.(event.target.value) + onInputChange?.(value) }} onKeyDown={(e) => { // Prevent form submission when the user presses Enter inside a combobox. diff --git a/app/util/str.spec.tsx b/app/util/str.spec.tsx index 1be63c62d8..f9e3c86697 100644 --- a/app/util/str.spec.tsx +++ b/app/util/str.spec.tsx @@ -13,6 +13,7 @@ import { commaSeries, extractText, kebabCase, + nameSyntax, titleCase, } from './str' @@ -110,3 +111,40 @@ describe('extractText', () => { expect(extractText('Some more text')).toBe('Some more text') }) }) + +describe('nameSyntax', () => { + it('converts to lowercase', () => { + expect(nameSyntax('Hello')).toBe('hello') + }) + + it('replaces spaces with dashes', () => { + expect(nameSyntax('Hello World')).toBe('hello-world') + }) + + it('removes non-alphanumeric characters', () => { + expect(nameSyntax('Hello, World!')).toBe('hello-world') + }) + + it('caps at 63 characters', () => { + expect(nameSyntax('aaa')).toBe('aaa') + expect(nameSyntax('aaaaaaaaa')).toBe('aaaaaaaaa') + expect(nameSyntax('a'.repeat(63))).toBe('a'.repeat(63)) + expect(nameSyntax('a'.repeat(64))).toBe('a'.repeat(63)) + }) + + it('can optionally start with numbers', () => { + expect(nameSyntax('123abc')).toBe('abc') + expect(nameSyntax('123abc', false)).toBe('abc') + expect(nameSyntax('123abc', true)).toBe('123abc') + }) + + it('can optionally start with a dash', () => { + expect(nameSyntax('-abc')).toBe('abc') + expect(nameSyntax('-abc', false)).toBe('abc') + expect(nameSyntax('-abc', true)).toBe('-abc') + }) + + it('does not complain when multiple dashes are present', () => { + expect(nameSyntax('a--b')).toBe('a--b') + }) +}) diff --git a/app/util/str.ts b/app/util/str.ts index a7620050c9..502a531c3c 100644 --- a/app/util/str.ts +++ b/app/util/str.ts @@ -58,6 +58,26 @@ export const titleCase = (text: string): string => { */ export const isAllZeros = (base64Data: string) => /^A*=*$/.test(base64Data) +/** Clean up text so that it conforms to Name field syntax rules: + * - lowercase only + * - no spaces + * - only letters/numbers/dashes allowed + * - capped at 63 characters + * By default, it must start with a letter; this can be overriden with the second argument, + * for contexts where we want to allow numbers at the start, like searching in comboboxes. + */ +export const nameSyntax = (text: string, allowNonLetterStart = false): string => { + const cleanedText = text + .toLowerCase() + .replace(/[\s_]+/g, '-') // Replace spaces and underscores with dashes + .replace(/[^a-z0-9-]/g, '') // Remove non-alphanumeric (or dash) characters + .slice(0, 63) // Limit string to 63 characters + if (allowNonLetterStart) { + return cleanedText + } + return cleanedText.replace(/^[^a-z]+/, '') // Remove any non-letter characters from the start +} + /** * Extract the string contents of a ReactNode, so <>This highlighted text becomes "This highlighted text" */ From a84758c03fd8749a24acd6b4482e61c8b764a0e9 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 13 Nov 2024 10:47:25 -0800 Subject: [PATCH 2/4] back out unintended Combobox changes --- app/ui/lib/Combobox.tsx | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx index fadc31c5f7..05523b7054 100644 --- a/app/ui/lib/Combobox.tsx +++ b/app/ui/lib/Combobox.tsx @@ -19,8 +19,6 @@ import { useEffect, useId, useState, type ReactNode, type Ref } from 'react' import { SelectArrows6Icon } from '@oxide/design-system/icons/react' -import { nameSyntax } from '~/util/str' - import { FieldLabel } from './FieldLabel' import { usePopoverZIndex } from './SideModal' import { TextInputHint } from './TextInput' @@ -121,9 +119,6 @@ export const Combobox = ({ if (allowArbitraryValues && !selectedItemValue) { setQuery('') } - if (selectedItemValue) { - setQuery(selectedItemValue) - } }, [allowArbitraryValues, selectedItemValue]) // If the user has typed in a value that isn't in the list, @@ -198,18 +193,13 @@ export const Combobox = ({ // selectedItemValue is displayed when the user can type in a new value. // Otherwise, use the provided selectedItemLabel displayValue={() => - selectedItemValue - ? allowArbitraryValues - ? selectedItemValue - : selectedItemLabel - : query + allowArbitraryValues ? selectedItemValue : selectedItemLabel } onChange={(event) => { - const value = nameSyntax(event.target.value, false) // updates the query state as the user types, in order to filter the list of items - setQuery(value) + setQuery(event.target.value) // if the parent component wants to know about input changes, call the callback - onInputChange?.(value) + onInputChange?.(event.target.value) }} onKeyDown={(e) => { // Prevent form submission when the user presses Enter inside a combobox. From bc4a054a07f5928263991bd8de867c7323aa0e04 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 13 Nov 2024 15:15:31 -0800 Subject: [PATCH 3/4] update function name to normalizeName --- app/components/form/fields/NameField.tsx | 4 +-- app/util/str.spec.tsx | 32 ++++++++++++------------ app/util/str.ts | 2 +- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/app/components/form/fields/NameField.tsx b/app/components/form/fields/NameField.tsx index f7d7ae7d79..75a2de693d 100644 --- a/app/components/form/fields/NameField.tsx +++ b/app/components/form/fields/NameField.tsx @@ -7,7 +7,7 @@ */ import type { FieldPath, FieldValues } from 'react-hook-form' -import { capitalize, nameSyntax } from '~/util/str' +import { capitalize, normalizeName } from '~/util/str' import { TextField, type TextFieldProps } from './TextField' @@ -30,7 +30,7 @@ export function NameField< required={required} label={label} name={name} - transform={(value) => nameSyntax(value)} + transform={(value) => normalizeName(value)} // https://www.stefanjudis.com/snippets/turn-off-password-managers/ data-1p-ignore data-bwignore diff --git a/app/util/str.spec.tsx b/app/util/str.spec.tsx index f9e3c86697..19fc93d56d 100644 --- a/app/util/str.spec.tsx +++ b/app/util/str.spec.tsx @@ -13,7 +13,7 @@ import { commaSeries, extractText, kebabCase, - nameSyntax, + normalizeName, titleCase, } from './str' @@ -112,39 +112,39 @@ describe('extractText', () => { }) }) -describe('nameSyntax', () => { +describe('normalizeName', () => { it('converts to lowercase', () => { - expect(nameSyntax('Hello')).toBe('hello') + expect(normalizeName('Hello')).toBe('hello') }) it('replaces spaces with dashes', () => { - expect(nameSyntax('Hello World')).toBe('hello-world') + expect(normalizeName('Hello World')).toBe('hello-world') }) it('removes non-alphanumeric characters', () => { - expect(nameSyntax('Hello, World!')).toBe('hello-world') + expect(normalizeName('Hello, World!')).toBe('hello-world') }) it('caps at 63 characters', () => { - expect(nameSyntax('aaa')).toBe('aaa') - expect(nameSyntax('aaaaaaaaa')).toBe('aaaaaaaaa') - expect(nameSyntax('a'.repeat(63))).toBe('a'.repeat(63)) - expect(nameSyntax('a'.repeat(64))).toBe('a'.repeat(63)) + expect(normalizeName('aaa')).toBe('aaa') + expect(normalizeName('aaaaaaaaa')).toBe('aaaaaaaaa') + expect(normalizeName('a'.repeat(63))).toBe('a'.repeat(63)) + expect(normalizeName('a'.repeat(64))).toBe('a'.repeat(63)) }) it('can optionally start with numbers', () => { - expect(nameSyntax('123abc')).toBe('abc') - expect(nameSyntax('123abc', false)).toBe('abc') - expect(nameSyntax('123abc', true)).toBe('123abc') + expect(normalizeName('123abc')).toBe('abc') + expect(normalizeName('123abc', false)).toBe('abc') + expect(normalizeName('123abc', true)).toBe('123abc') }) it('can optionally start with a dash', () => { - expect(nameSyntax('-abc')).toBe('abc') - expect(nameSyntax('-abc', false)).toBe('abc') - expect(nameSyntax('-abc', true)).toBe('-abc') + expect(normalizeName('-abc')).toBe('abc') + expect(normalizeName('-abc', false)).toBe('abc') + expect(normalizeName('-abc', true)).toBe('-abc') }) it('does not complain when multiple dashes are present', () => { - expect(nameSyntax('a--b')).toBe('a--b') + expect(normalizeName('a--b')).toBe('a--b') }) }) diff --git a/app/util/str.ts b/app/util/str.ts index 502a531c3c..bd5f29b152 100644 --- a/app/util/str.ts +++ b/app/util/str.ts @@ -66,7 +66,7 @@ export const isAllZeros = (base64Data: string) => /^A*=*$/.test(base64Data) * By default, it must start with a letter; this can be overriden with the second argument, * for contexts where we want to allow numbers at the start, like searching in comboboxes. */ -export const nameSyntax = (text: string, allowNonLetterStart = false): string => { +export const normalizeName = (text: string, allowNonLetterStart = false): string => { const cleanedText = text .toLowerCase() .replace(/[\s_]+/g, '-') // Replace spaces and underscores with dashes From cc6c7911b44c06cbde53a628a87f21b3adecb110 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 13 Nov 2024 15:16:28 -0800 Subject: [PATCH 4/4] rename const --- app/util/str.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/util/str.ts b/app/util/str.ts index bd5f29b152..1f431e1d58 100644 --- a/app/util/str.ts +++ b/app/util/str.ts @@ -67,15 +67,15 @@ export const isAllZeros = (base64Data: string) => /^A*=*$/.test(base64Data) * for contexts where we want to allow numbers at the start, like searching in comboboxes. */ export const normalizeName = (text: string, allowNonLetterStart = false): string => { - const cleanedText = text + const normalizedName = text .toLowerCase() .replace(/[\s_]+/g, '-') // Replace spaces and underscores with dashes .replace(/[^a-z0-9-]/g, '') // Remove non-alphanumeric (or dash) characters .slice(0, 63) // Limit string to 63 characters if (allowNonLetterStart) { - return cleanedText + return normalizedName } - return cleanedText.replace(/^[^a-z]+/, '') // Remove any non-letter characters from the start + return normalizedName.replace(/^[^a-z]+/, '') // Remove any non-letter characters from the start } /**