-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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.
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
Components: Use useStoreState()
instead of store.useState()
#64648
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,11 @@ | ||
/** | ||
* WordPress dependencies | ||
* External dependencies | ||
*/ | ||
import { useStoreState } from '@ariakit/react'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { forwardRef } from '@wordpress/element'; | ||
|
||
/** | ||
|
@@ -22,13 +26,13 @@ export const TabPanel = forwardRef< | |
ref | ||
) { | ||
const context = useTabsContext(); | ||
const selectedId = useStoreState( context?.store, 'selectedId' ); | ||
if ( ! context ) { | ||
warning( '`Tabs.TabPanel` must be wrapped in a `Tabs` component.' ); | ||
return null; | ||
} | ||
const { store, instanceId } = context; | ||
const instancedTabId = `${ instanceId }-${ tabId }`; | ||
const selectedId = store.useState( ( state ) => state.selectedId ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved up above the condition to fulfill the rules of hooks. Simplified the callback syntax. |
||
|
||
return ( | ||
<StyledTabPanel | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,8 @@ | |
*/ | ||
import clsx from 'clsx'; | ||
// TODO: use the @wordpress/components one once public | ||
// eslint-disable-next-line no-restricted-imports | ||
import { useStoreState } from '@ariakit/react'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @WordPress/gutenberg-components WDYT - should we re-export this from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess it depends on what we decide around exposing stores in #63704. Better hold off for now |
||
// Import CompositeStore type, which is not exported from @wordpress/components. | ||
// eslint-disable-next-line no-restricted-imports | ||
import type { CompositeStore } from '@ariakit/react'; | ||
|
@@ -359,10 +361,11 @@ export default function ViewList< Item >( props: ViewListProps< Item > ) { | |
|
||
const store = useCompositeStore( { | ||
defaultActiveId: getItemDomId( selectedItem ), | ||
} ); | ||
} ) as CompositeStore; // TODO, remove once composite APIs are public | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Necessary for the |
||
|
||
// Manage focused item, when the active one is removed from the list. | ||
const isActiveIdInList = store.useState( | ||
const isActiveIdInList = useStoreState( | ||
store, | ||
( state: { items: any[]; activeId: any } ) => | ||
state.items.some( | ||
( item: { id: any } ) => item.id === state.activeId | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason for using a separate import instead of
Ariakit.useStoreState
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, no. I think it was caused by the automated way I addressed these changes. Let me fix these in a follow-up PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#67818 - thanks for spotting @Mamaduka 🙌