Skip to content

Wrong argument is passed for removeItemLabelText function #1296

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.

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

Open
m-vo opened this issue Apr 3, 2025 · 6 comments
Open

Wrong argument is passed for removeItemLabelText function #1296

m-vo opened this issue Apr 3, 2025 · 6 comments
Labels

Comments

@m-vo
Copy link

m-vo commented Apr 3, 2025

When the removeItemLabelText config option is defined as a function, it will get invoked with the element's value, not the text (label). As this is a helper text for users, I don't think you ever want the (technical) values.

Also, see the default:

  removeItemLabelText: (value) => `Remove item: ${value}`,

Note, that you cannot get the associated label to the value in the function as you may not have a reference to the instance at this point.

Affected version

  • Version: v11.1
  • Introduced with 8f16514
@m-vo m-vo added the bug label Apr 3, 2025
@zoglo
Copy link
Contributor

zoglo commented Apr 3, 2025

A workaround would be something like this but I don't think we should parse the select again in the callback and handle it internally 🤔

removeItemLabelText: function (value) {
    const option = Array.from(element.options).filter(opt => opt.value === value)[0];
    return `Remove item: ${option?.textContent || value}`,
},

@Xon
Copy link
Collaborator

Xon commented Apr 3, 2025

Adding an choice/option argument filled similar to how the various Events would be relatively easy to add.

A workaround would be something like this but I don't think we should parse the select again in the callback and handle it internally 🤔

removeItemLabelText: function (value) {
const option = Array.from(element.options).filter(opt => opt.value === value)[0];
return Remove item: ${option?.textContent || value},
},

This isn't going to work as expected as value is escaped by default. You'ld need to use the 2nd argument (basically valueRaw) to lookup the thing by value.

@m-vo
Copy link
Author

m-vo commented Apr 3, 2025

But can't we just add the "thing" as a 3rd argument?

Edit: I realised that is probably what you meant by "Adding an choice/option argument filled similar to how the various Events would be relatively easy to add.".

@RichardDavies
Copy link

I ran into this issue while attempting to implement Choices. It would be great to have it fixed because it's a major accessibility issue and a violation of https://www.w3.org/WAI/WCAG22/Techniques/general/G211.html.

As the OP mentioned, the default behavior should be to use the item's name in the remove button label since that is the text that is presented to the user when they add/view the item in the list. The system value is never presented to the user and is likely unknown/unfamiliar to them.

@RichardDavies
Copy link

RichardDavies commented May 7, 2025

As a workaround, I was able to use this code (adapted from @zoglo's suggestion) to set the remove button label to the item's name.

removeItemLabelText: function (value, valueRaw) {
  const options = document.querySelectorAll('option');
  const option = Array.from(options).filter(opt => opt.value === valueRaw).filter(opt => opt.selected)[0];
  const label = option.textContent;
  return `Remove item: ${label}`;
},

Note that this isn't perfect. It may produce unexpected results if you have multiple <select>s on the same page that have options with the same value.

@jamesperrin
Copy link

jamesperrin commented May 19, 2025

Great suggestions by @zoglo and @RichardDavies.

The code worked even when there are multiple select elements. However, the issue I ran into was the code would query the web page document for all options every time a User choose an item.

Here is my band aid solution attempt that only queries the web page document once for all Choice.js options..

The code initializes a variable called choiceOptions. When the function getChoiceOptionLabel is invoked, it returns the matching option label value from the variable choiceOptions. This helps to resolve the web accessibility issue for Technique G211: Matching the accessible name to the visible label.

IIFE to capture all select options only used with Choice.js.

const choiceOptions = (() => {
	// Initialize an array to hold options
	let choiceOptionsArr = [];

	// Function to capture select elements options
	const captureChoiceOptions = () => {
		// Capture all options where select elements contains the CSS class 'js-choice'.
		const options = document.querySelectorAll('select[class*="js-choice"] option');

		// Convert NodeList to Array and merge with choiceOptionsArr
		choiceOptionsArr = choiceOptionsArr.concat(Array.from(options));
	};

	// Capture options when the closure is initialized
	captureChoiceOptions();

	// Return the array of captured options
	return choiceOptionsArr;
})();

Function to retrieve matching option label.

getChoiceOptionLabel(options, valueRaw) => {
	// Filter the options to find the one with the matching value
	try {
		const option = Array.from(options).filter(opt => opt.value === valueRaw).filter(opt => opt.selected)[0];
		return option.textContent;
	} catch (error) {
		console.error('Error in getChoiceOptionLabel:', error);
		return valueRaw;
	}
}

Initialize ChoiceJS on DOMContentLoaded event.

document.addEventListener('DOMContentLoaded', function () {
	// Default ChoiceJS options
	const choicejsDefaultOptions = {
		duplicateItemsAllowed: false,
		removeItemButton: true,
		removeItemIconText: function (value, valueRaw) {
			const label = getChoiceOptionLabel(choiceOptions, valueRaw);
			return `Remove item: ${label}`;
		},
		removeItemLabelText: function (value, valueRaw) {
			const label = getChoiceOptionLabel(choiceOptions, valueRaw);
			return `Remove item: ${label}`;
		},
	};

	// Fruits select element.
	const fruitsElement = document.querySelector('.js-choice__Fruits');

	// Initialize Fruits ChoiceJS
	const fruitsChoicesJS = new Choices(fruitsElement, {
		...choicejsDefaultOptions,
		labelId: fruitsElement.id,
		placeholderValue: fruitsElement.name,
	});
});

Open to feedback and suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants