Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hosting Command Palette: Trim double quotes from selected items if present #84373

Merged
merged 4 commits into from
Nov 23, 2023
Merged

Hosting Command Palette: Trim double quotes from selected items if present #84373

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
80e13a8
Remove double quotes from site title if present
Nov 21, 2023
d5e8cd8
Remove unnecessary trim
Nov 21, 2023
fd8dae6
Hosting Command Palette: fix active descendant id for reserved charac…
sejas Nov 22, 2023
ad6e24a
Merge branch 'trunk' of github.com:Automattic/wp-calypso into fix/tri…
sejas Nov 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions client/components/command-pallette/wpcom-command-pallette.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,23 @@ interface CommandInputProps {
function CommandInput( { isOpen, search, setSearch }: CommandInputProps ) {
const commandMenuInput = useRef< HTMLInputElement >( null );
const _value = useCommandState( ( state ) => state.value );
const sanitizedValue = useMemo( () => {
const removedQuotesValue = _value.replace( /"/g, '' ); // Remove double quotes from any selected items before processing input
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use escapeAttribute from @wordpress/escape-html? https://developer.wordpress.org/block-editor/reference-guides/packages/packages-escape-html/#escapeattribute

Or how do we match the sanitization the library is already applying, to make sure our value exactly matches what's rendered on the page?

Also, what other characters might cause problems?

Copy link
Member

@sejas sejas Nov 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, what other characters might cause problems?

Yesterday reviewing the PR I found that the double quotes character is problematic. The rest worked fine.

return removedQuotesValue;
}, [ _value ] );

const selectedItemId = useMemo( () => {
const item = document.querySelector( `[cmdk-item=""][data-value="${ _value }"]` );
const item = document.querySelector( `[cmdk-item=""][data-value="${ sanitizedValue }"]` );
return item?.getAttribute( 'id' );
}, [ _value ] );
}, [ sanitizedValue ] );

useEffect( () => {
// Focus the command palette input when mounting the modal.
if ( isOpen ) {
commandMenuInput.current?.focus();
}
}, [ isOpen ] );

return (
<Command.Input
ref={ commandMenuInput }
Expand Down
Loading