Skip to content

Commit

Permalink
Remove unused optionRenderer prop on dropdown's Option component (#1794)
Browse files Browse the repository at this point in the history
## Summary:

As I was working in the code I noticed that the `optionRenderer` prop is never passed to the Option nor OptionGroup components. So I'm removing it. 

Issue: "none"

## Test plan:

`yarn typecheck`
`yarn test`
`rg optionRenderer` (in hosting applications - webapp, mobile)

Author: jeremywiebe

Reviewers: handeyeco, benchristel, nishasy, mark-fitzgerald, catandthemachines, anakaren-rojas

Required Reviewers:

Approved By: handeyeco

Checks: ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1794
  • Loading branch information
jeremywiebe authored Oct 25, 2024
1 parent 923d0be commit 9dd0f8c
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 42 deletions.
5 changes: 5 additions & 0 deletions .changeset/perfect-guests-raise.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus-editor": patch
---

Remove unused prop, optionRenderer, from dropdown-option component
55 changes: 13 additions & 42 deletions packages/perseus-editor/src/components/dropdown-option.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,6 @@ import ReactDOM from "react-dom";

import {focusWithChromeStickyFocusBugWorkaround} from "./util";

type OptionRenderer = (
children: React.ReactElement<any> | null | undefined,
value: string,
selected: boolean,
disabled?: boolean,
) => React.ReactElement<any>;

const {Icon} = components;
const {colors} = globalStyles;

Expand All @@ -43,8 +36,6 @@ type Props = {
disabled?: boolean;
// An event when an option is clicked
onClick?: () => void;
// An optional rendering function to render the details of the option
optionRenderer?: OptionRenderer;
// An optional function to call when the dropdown should close
// Only relevant if the Option is inside of a dropdown menu
onDropdownClose?: () => void;
Expand Down Expand Up @@ -105,7 +96,6 @@ class Option extends React.Component<Props> {
value,
onClick,
children,
optionRenderer,
disabled,
hideFocusState,
testId,
Expand Down Expand Up @@ -137,35 +127,20 @@ class Option extends React.Component<Props> {
aria-label={ariaLabel}
data-testid={testId}
>
{optionRenderer &&
optionRenderer(
/**
* This expects a `React.Element<>` but `children` is a
* `React.Node`. We can convert a `React.Node` to a
* `React.Element<>` by wrapping it in a fragment.
*/
<>{children}</>,
value || "",
selected || false,
disabled,
<span
className={css(
styles.option,
selected && styles.optionSelected,
disabled && styles.optionDisabled,
)}

{!optionRenderer && (
<span
className={css(
styles.option,
selected && styles.optionSelected,
disabled && styles.optionDisabled,
)}
>
{children}
{selected && (
<span className={css(styles.check)}>
<Icon icon={check} />
</span>
)}
</span>
)}
>
{children}
{selected && (
<span className={css(styles.check)}>
<Icon icon={check} />
</span>
)}
</span>
</button>
);
}
Expand All @@ -182,8 +157,6 @@ class OptionGroup extends React.Component<{
children?: Array<React.ReactElement<React.ComponentProps<typeof Option>>>;
// The currently selected options
selectedValues: Array<string>;
// An optional rendering function to render the details of the options
optionRenderer?: OptionRenderer;
// An override to skip the bit of top/bottom spacing around the group
noMargin?: boolean;
// An optional function to call when the dropdown should close
Expand All @@ -207,7 +180,6 @@ class OptionGroup extends React.Component<{
children,
onSelected,
selectedValues,
optionRenderer,
noMargin,
onDropdownClose,
hideFocusState,
Expand Down Expand Up @@ -239,7 +211,6 @@ class OptionGroup extends React.Component<{
selected: selected,
// @ts-expect-error - TS2532 - Object is possibly 'undefined'.
onClick: () => onSelected(child.props.value),
optionRenderer: optionRenderer,
ref: reference,
onDropdownClose: onDropdownClose,
hideFocusState: hideFocusState,
Expand Down

0 comments on commit 9dd0f8c

Please sign in to comment.