Move shared vislib components into Charts plugin

[DianaDerevyankina]

Closes #56310

Summary

Moved some components/types from the vis_type_vislib plugin that are used in other plugins to the Charts plugin.

Checklist

Delete any items that are not applicable to this PR.

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• This was checked for keyboard-only and screenreader accessibility
• This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
• This was checked for cross-browser compatibility, including a check against IE11

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[cla-checker-service]

💚 CLA has been signed

[DianaDerevyankina]

cla/check

[elasticmachine]

Pinging @elastic/kibana-app (Team:KibanaApp)

[flash1293]

Tested in Chrome and everything works as expected, left one nit about naming

[flash1293]

I think we can rename this to ColorSchemaParams it's not really bound to vislib anymore.

[maryia-lapata]

I think we can use enum here:

export enum ColorModes {
  BACKGROUND = 'Background',
  LABELS = 'Labels',
  NONE = 'None',
}

and for Rotates as well.

[flash1293]

I think @nickofthyme introduced these but I forgot what the advantage over enums was. Could you explain, Nick?

[nickofthyme]

The short answer, nothing really.

In elastic-charts (elastic/elastic-charts#266) we only use object enums because of the bloat the standard ts enums create in the final transpiled javascript.

The regular enum would produce something like

enum Pet1 {
  Cat = 'Cat',
  Dog = 'Dog',
}

// transpiles to this
var Pet1;
(function (Pet1) {
    Pet1["Cat"] = "Cat";
    Pet1["Dog"] = "Dog";
})(Pet1 || (Pet1 = {}));

where a much cleaner output is

const Pet2 = Object.freeze({
  Cat: 'Cat' as 'Cat',
  Dog: 'Dog' as 'Dog',
})
type Pet2 = typeof Pet2.Cat | typeof Pet2.Dog;

// transpiles to this
const Pet2 = Object.freeze({
    Cat: 'Cat',
    Dog: 'Dog',
});

Using a const enum would be clearer but they are not yet supported in babel (babel/babel#8741)

[maryia-lapata]

@sulemanof I noticed that there are two version of useValidation: in vis_default_editor and in vis_type_vislib. I believe it makes sense to leave just one version (I guess from vis_default_editor). What do you think?

[sulemanof]

These are different, check out interfaces there.
So the second one respects particular paramName to be invalid

[maryia-lapata]

I mean to use the version from vis_default_editor and a bit change useValidation usage in RequiredNumberInputOption:

  const setControlValidity = useCallback((isValidControl: boolean) => {
    setValidity(paramName, isValidControl);
  }, [setValidity]);
  useValidation(setControlValidity, isValid);

Also the RequiredNumberInputOption control is currently used in vis_type_vislib only. So I think we don't need to move it to charts plugin.

[flash1293]

I agree it's confusing to have to slightly different helpers with different names. Let's consolidate into one if that's possible without major changes.

About RequiredNumberInputOption: OTOH it might be helpful somewhere else too later, not really a preference here.

[sulemanof]

Makes sense.
If it is the only usage, I would also keep the only useEffect here.
But I'm fine with both.

[sulemanof]

About RequiredNumberInputOption: OTOH it might be helpful somewhere else too later, not really a preference here.

Yep, there is also an issue created for this purpose

[nickofthyme]

Code changes LGTM. Verified PR locally on Chrome.

Verified the following with sample datasets:

• ColorSchemaOptions and ColorRanges in metrics vis
• BasicOptions, RequiredNumberInputOption and ThresholdPanel in vertical bar vis
• All other moved components and types in panel settings.

[maryia-lapata]

nit: we can combine line 34:

Suggested change

	SwitchOption,
	} from '../../../../../../../plugins/charts/public';
	SwitchOption,
	ColorSchemas,
	} from '../../../../../../../plugins/charts/public';

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: f7fcf67

History

• 💚 Build #40111 succeeded bc4807e
• 💔 Build #40097 failed 8824aa4
• 💚 Build #39826 succeeded 0b50ca8
• 💔 Build #39638 failed af78c9a
• 💔 Build #39605 failed ac41523

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream