Skip to content

Commit

Permalink
Add children to Dropdown props interface (#171)
Browse files Browse the repository at this point in the history
* Add children to Dropdown props interface

While attempting to import the `<Dropdown>` component from Flame in the new insights app (https://github.com/lightspeed-hospitality/lighthouse-insights), we ran into a Typescript issue where the app wouldn't compile because `children` was not a valid prop in the `Dropdown` interface.

The insights app is using React 18 and in that version React.FC no longer includes children by default.
- https://stackoverflow.com/a/71809927/16826114
- https://solverfox.dev/writing/no-implicit-children/

So any Flame components that use React.FC and a children prop will probably need to be updated to be compatible with React 18 and TS. For now we're making this one change in this one component we need as a way to kick the tires on contributing to Flame and to make sure it works as we expect.

* Update CHANGELOG.md

* Add documentation about using `yarn link`

* Update CONTRIBUTING.md
  • Loading branch information
ls-andrew-borstein authored Oct 13, 2022
1 parent 6055868 commit f38c060
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 0 deletions.
14 changes: 14 additions & 0 deletions .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,20 @@ yarn dev

This will install/update dependencies, including `packages/` ones, and launch Storybook on [http://localhost:6006/](http://localhost:6006/).

#### Link a local version of @lightspeed/flame to your project

If you want to test your changes in a local project, you can link your local version of `@lightspeed/flame` using [yarn link](https://classic.yarnpkg.com/en/docs/cli/link/), e.g.

```sh
# Link the react package
cd packages/flame/dist
yarn link
cd /path/to/your/project
yarn link "@lightspeed/flame"
```

Note that in the above example you need to run `yarn link` inside the `dist` folder of a given package. If you're not seeing local changes reflected in your project, you may need to run `yarn build` inside the `packages/flame` folder and then restart your project's web server or [typescript server](https://tinytip.co/tips/vscode-restart-ts).

#### Run tests

```sh
Expand Down
7 changes: 7 additions & 0 deletions packages/flame/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

Refer to the [CONTRIBUTING guide](https://github.com/lightspeed/flame/blob/master/.github/CONTRIBUTING.md) for more info.

## [Unreleased]

### Added

- Add children to Dropdown props interface ([#171](https://github.com/lightspeed/flame/pull/171))
- Update CONTRIBUTING.md about using `yarn link` ([#172](https://github.com/lightspeed/flame/pull/171))

## 2.4.2 - 2022-10-12

### Added
Expand Down
1 change: 1 addition & 0 deletions packages/flame/src/Dropdown/Dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type Placement = 'start' | 'center' | 'end' | PopperPlacement;

interface Props extends Merge<PopoverContainerProps, Omit<ButtonProps, 'onClick'>> {
buttonContent: React.ReactNode;
children: React.ReactNode;
initiallyOpen?: boolean;
placement?: Placement;
onClick?: (toggle: () => void, event: React.MouseEvent<HTMLButtonElement, MouseEvent>) => void;
Expand Down

0 comments on commit f38c060

Please sign in to comment.