Improvements to the TypeaheadSelect template

mvollmer commented 2 weeks ago

closes# https://github.com/patternfly/patternfly-react/issues/11183 Here are a few small changes to the TypeaheadSelect that we could use in Cockpit.

Our linter doesn't like the "any" type.
TypeScript actually reports a typing error for the "isDisabled" property, which seems easily fixed.
Not all our typeahead users specify a "onClearSelection" function, and for those the "X" button was still there but did nothing. I think it is better to not show that button in that case.

Thanks!

patternfly-build commented 2 weeks ago

Preview: https://patternfly-react-pr-11153.surge.sh

A11y report: https://patternfly-react-pr-11153-a11y.surge.sh

mvollmer commented 2 weeks ago

TypeScript actually reports a typing error for the "isDisabled" property, which seems easily fixed.

Or not... :-) I didn't read the error message carefully enough, which is:

# pkg/lib/cockpit-components-typeahead-select.tsx:406:6 - error TS2375: Type '{ children: Element; className?: string; isExpanded: boolean; isDisabled: boolean | undefined; isFullHeight?: boolean; isFullWidth: boolean; splitButtonOptions?: SplitButtonOptions; ... 281 more ...; ref: Ref<...>; }' is not assignable to type 'MenuToggleProps' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
#   Types of property 'isDisabled' are incompatible.
#     Type 'boolean | undefined' is not assignable to type 'boolean'.
#       Type 'undefined' is not assignable to type 'boolean'.

I have pushed a hopefully better fix.

Why do you not see this error? Do we have different versions of MenuToggleProps?

kmcfaul commented 1 week ago

Not sure why we aren't seeing this error. Are you running with strict mode maybe?

mvollmer commented 5 days ago

Are you running with strict mode maybe?

Yes.

We have copied the template into our sources so that we can make changes like in this PR, and we lint and type-check it like the rest of the Cockpit code. If we would import the template from @patternfly/react-templates, I guess we would not see that error.

patternfly-build commented 2 days ago

Your changes have been released in:

@patternfly/react-docs@6.4.16
@patternfly/react-templates@1.1.9

Thanks for your contribution! :tada:

patternfly / patternfly-react

Improvements to the TypeaheadSelect template #11153