Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Divider: migrate from reakit to @ariakit/react #55622

Merged
merged 6 commits into from
Nov 6, 2023
Merged

Divider: migrate from reakit to @ariakit/react #55622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
642916d
`components/Divider`: refactor from `reakit` to `@ariakit/react`
flootr Oct 25, 2023
3f9a6a4
add changelog entry
flootr Oct 31, 2023
6f6ad1f
types: exlude `as` & `render` from `SeparatorProps`
flootr Nov 2, 2023
827ae01
storybook: disable `wrapElement` controls
flootr Nov 6, 2023
d23968c
storybook: remove `ref` from props table
flootr Nov 6, 2023
9dfeb1b
move changelog entry to UNRELEASED
flootr Nov 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/components/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Internal

- Migrate `Divider` from `reakit` to `ariakit` ([#55622](https://github.com/WordPress/gutenberg/pull/55622))

## 25.11.0 (2023-11-02)

### Enhancements
Expand Down
6 changes: 3 additions & 3 deletions packages/components/src/divider/component.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* External dependencies
*/
// eslint-disable-next-line no-restricted-imports
import { Separator } from 'reakit';
import * as Ariakit from '@ariakit/react';
import type { ForwardedRef } from 'react';

/**
Expand All @@ -20,8 +20,8 @@ function UnconnectedDivider(
const contextProps = useContextSystem( props, 'Divider' );

return (
<Separator
as={ DividerView }
<Ariakit.Separator
render={ <DividerView /> }
{ ...contextProps }
ref={ forwardedRef }
/>
Expand Down
8 changes: 8 additions & 0 deletions packages/components/src/divider/stories/index.story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ const meta: Meta< typeof Divider > = {
marginEnd: {
control: { type: 'text' },
},
wrapElement: {
control: { type: null },
},
ref: {
table: {
disable: true,
},
},
},
parameters: {
controls: { expanded: true },
Expand Down
4 changes: 2 additions & 2 deletions packages/components/src/divider/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* External dependencies
*/
// eslint-disable-next-line no-restricted-imports
import type { SeparatorProps } from 'reakit';
import type { SeparatorProps } from '@ariakit/react';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the SeparatorProps from ariakit include the as and render props, which we don't want to expose.

We can easily fix this by adding 'as' and 'render' to the list of omitted props on line 14, but it would be also interesting to know if there;s any particular reason why previously we were not picking up the as prop from SeparatorProps, and now we are.

Copy link
Contributor Author

@flootr flootr Nov 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I applied your suggestions via 7d0b7ce – I'm a bit unsure about the rationale you mentioned.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies in case I wasn't clear. What I was trying to say is that I was curious to know the reason why previously, when using reakit types, the as prop was not being picked up from SeparatorProps. I guess the answer is simply because SeparatorProps did not include the as prop .


/**
* Internal dependencies
Expand All @@ -11,7 +11,7 @@ import type { SpaceInput } from '../utils/space';

export type DividerProps = Omit<
SeparatorProps,
'children' | 'unstable_system' | 'orientation'
'children' | 'unstable_system' | 'orientation' | 'as' | 'render'
> & {
/**
* Adjusts all margins on the inline dimension.
Expand Down