List of filtered values should show the formatted numeric value not the source data

[jansiegel]

Context

This PR:

• Adds a new modifyFiltersMultiSelectValue hook
• Makes the filters' multi-selection module utilize the new hook to render the entry list
• Utilizes the modifyFiltersMultiSelectValue to display the rendered values for the numeric-typed cells

How has this been tested?

Added new test cases.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature or improvement (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Additional language file or change to the existing one (translations)

Related issue(s):

1. handsontable/dev-handsontable#1267

Affected project(s):

• handsontable
• @handsontable/angular
• @handsontable/react
• @handsontable/vue
• @handsontable/vue3

Checklist:

• I have reviewed the guidelines about Contributing to Handsontable and I confirm that my code follows the code style of this project.
• I have signed the Contributor License Agreement
• My change requires a change to the documentation.

[github-actions]

Launch the local version of documentation by running:

npm run docs:review 8f6a5f51261db15b47798bdc4b9b55224139948a

[budnix]

I'm not convinced about that displayValue option. I know that the purpose of this task is to add support for modifying filter values only for numeric values, but with this shape, it isn't very clear.
The question that comes to my mind or limitation that I found:

• The value can not be changed in the cells function. The displayValue property is always overwritten in the numeric renderer;
• When the displayValue property does not exist, the Filters plugin generates one by calling the numeric renderer by itself. So what is this option for? Kind of read-only and for numeric columns only?
• Maybe a better idea would be creating renderers for Filters. The same goes for cells. For example, filtersRenderer option, or so?

[jansiegel]

I went through several approaches to this task, but I'll take any suggestions. As for the questions:

• The value can not be changed in the cells function. The displayValue property is always overwritten in the numeric renderer;

Yes. The spec requires the multi-selection component in the Filters dropdown to display the same value that's being rendered in the cells. As we cannot know what the renderer with generate, there's no way to assign this value before it's triggered.

• When the displayValue property does not exist, the Filters plugin generates one by calling the numeric renderer by itself. So what is this option for? Kind of read-only and for numeric columns only?

I had to add this logic because the Filters dropdown display all unique values in the table, and the renderers are run just for the visible part of the dataset. Without this logic, some of the values were being displayed in their "source" form, and some in the "rendered" form.

• Maybe a better idea would be creating renderers for Filters. The same goes for cells. For example, filtersRenderer option, or so?

We can do that, but according the the spec, in this case the filtersRenderer should be exact same as the regular renderer (and for the other cell types the filtersRenderer should return the cell's source value...?)

[budnix]

Maybe I'd suggest adding a new hook for the MultipleSelect component of the Filters plugin. Let's name it modifyFiltersMultipleSelectValue hook, for example. This hook would be responsible only for modifying the values presented by that component. There would be one hook for all cell types.

Now, depending on the feature needs you can pass as many arguments as you wish. Usage example:

modifyFiltersMultipleSelectValue(sourceValue, cellType, cellProperties) => {
  let newValue = sourceValue;

  if (cellType === 'numeric') {
    if (isNumeric(newValue)) { // HERE I'd copy the logic from the renderer, or I can suggest moving it to a separate helper
      const numericFormat = cellProperties.numericFormat;
      const cellCulture = numericFormat && numericFormat.culture || '-';
      const cellFormatPattern = numericFormat && numericFormat.pattern;
      const className = cellProperties.className || '';
      const classArr = className.length ? className.split(' ') : [];

      if (typeof cellCulture !== 'undefined' && !numbro.languages()[cellCulture]) {
        const shortTag = cellCulture.replace('-', '');
        const langData = numbro.allLanguages ? numbro.allLanguages[cellCulture] : numbro[shortTag];

        if (langData) {
          numbro.registerLanguage(langData);
        }
      }

      numbro.setLanguage(cellCulture);

      newValue = numbro(newValue).format(cellFormatPattern || '0');
    } 
  }
  // PLACE FOR OTHER CELL TYPES

  return newValue;
}

[budnix]

It looks good. I left one comment with suggestions to be considered. And I wonder, would it be possible to pass a complete cellMeta object as a second argument? It would be better for the user not to have to figure out a new data structure. They already know/use cellMeta objects in other hooks or methods.

[budnix]

Ah, and I almost forgot. Can you add a TypeScript definition for modifyFiltersMultiSelectValue hook? 😄