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

[Block Library - Query]: Allow term addition from user case-insensitive input #31301

Merged
merged 2 commits into from
May 15, 2021
Merged

[Block Library - Query]: Allow term addition from user case-insensitive input #31301

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3b74b83
[Block Library - Query]: Allow term addition from user case-insensiti…
ntsekouras Apr 28, 2021
af7ba55
fix typo
ntsekouras May 14, 2021
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
37 changes: 32 additions & 5 deletions packages/block-library/src/query/edit/query-inspector-controls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,28 @@ const CreateNewPostLink = ( { type } ) => {
);
};

// Helper function to get the term id based on user input in terms `FormTokenField`.
const getTermIdByTermValue = ( termsMappedByName, termValue ) => {
// First we check for exact match by `term.id` or case sensitive `term.name` match.
const termId = termValue?.id || termsMappedByName[ termValue ]?.id;
if ( termId ) return termId;
/**
* Here we make an extra check for entered terms in a non case sensitive way,
* to match user expectations, due to `FormTokenField` behaviour that shows
* suggestions which are case insensitive.
*
* Although WP tries to discourage users to add terms with the same name (case insensitive),
* it's still possible if you manually change the name, as long as the terms have different slugs.
* In this edge case we always apply the first match from the terms list.
*/
const termValueLower = termValue.toLocaleLowerCase();
for ( const term in termsMappedByName ) {
if ( term.toLocaleLowerCase() === termValueLower ) {
return termsMappedByName[ term ].id;
}
}
};

export default function QueryInspectorControls( {
attributes: { query, layout },
setQuery,
Expand Down Expand Up @@ -114,11 +136,16 @@ export default function QueryInspectorControls( {
};
// Handles categories and tags changes.
const onTermsChange = ( terms, queryProperty ) => ( newTermValues ) => {
const termIds = newTermValues.reduce( ( accumulator, termValue ) => {
const termId = termValue?.id || terms.mapByName[ termValue ]?.id;
if ( termId ) accumulator.push( termId );
return accumulator;
}, [] );
const termIds = Array.from(
newTermValues.reduce( ( accumulator, termValue ) => {
const termId = getTermIdByTermValue(
terms.mapByName,
termValue
);
if ( termId ) accumulator.add( termId );
return accumulator;
}, new Set() )
);
Comment on lines +139 to +148
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, I compared the performance of this "reduce to a set then cast back to array" approach with Lodash's uniq, and uniq performs better, even though the performance is more variable:

Captura de ecrã 2021-05-14, às 12 24 16

This is just an interesting tidbit, not something that needs to be fixed!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting indeed... I'll have to look into this a bit in general, as I'm quite fond of reduce 😄

setQuery( { [ queryProperty ]: termIds } );
};
const onCategoriesChange = onTermsChange( categories, 'categoryIds' );
Expand Down