-
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
Add reusable preferences modal to interface package. #39153
Changes from all commits
a7ee163
bf9b839
3c08ed5
4d8563a
f58d859
68918a0
6d11d8f
a7daf1e
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 |
---|---|---|
@@ -0,0 +1,42 @@ | ||
#__unstablePreferencesModalBaseOption | ||
|
||
`__unstablePreferencesModalBaseOption` renders a toggle meant to be used with `PreferencesModal`. | ||
|
||
This component implements a `ToggleControl` component from the `@wordpress/components` package. | ||
|
||
**It is an unstable component so is subject to breaking changes at any moment. Use at own risk.** | ||
|
||
## Example | ||
|
||
```jsx | ||
function MyEditorPreferencesOption() { | ||
return ( | ||
<__unstablePreferencesModalBaseOption | ||
label={ label } | ||
isChecked={ isChecked } | ||
onChange={ setIsChecked } | ||
> | ||
{ isChecked !== areCustomFieldsEnabled && ( | ||
<CustomFieldsConfirmation willEnable={ isChecked } /> | ||
) } | ||
</__unstablePreferencesModalBaseOption> | ||
) | ||
} | ||
``` | ||
|
||
## Props | ||
|
||
### help | ||
### label | ||
### isChecked | ||
### onChange | ||
|
||
These props are passed directly to ToggleControl, so see [ToggleControl readme](https://github.com/WordPress/gutenberg/blob/trunk/packages/components/src/toggle-control/README.md) for more info. | ||
|
||
### children | ||
|
||
Components to be rendered as content. | ||
|
||
- Type: `Element` | ||
- Required: No. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { ToggleControl } from '@wordpress/components'; | ||
|
||
function BaseOption( { help, label, isChecked, onChange, children } ) { | ||
return ( | ||
<div className="interface-preferences-modal__option"> | ||
<ToggleControl | ||
help={ help } | ||
label={ label } | ||
checked={ isChecked } | ||
onChange={ onChange } | ||
/> | ||
{ children } | ||
</div> | ||
); | ||
} | ||
|
||
export default BaseOption; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
.interface-preferences-modal__option { | ||
.components-base-control { | ||
.components-base-control__field { | ||
align-items: center; | ||
display: flex; | ||
margin-bottom: 0; | ||
|
||
& > label { | ||
flex-grow: 1; | ||
padding: 0.6rem 0 0.6rem 10px; | ||
} | ||
} | ||
} | ||
|
||
.components-base-control__help { | ||
margin: -$grid-unit-10 0 $grid-unit-10 58px; | ||
font-size: $helptext-font-size; | ||
font-style: normal; | ||
color: $gray-700; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
|
||
`PreferencesModalSection` renders a section (as a fieldset) meant to be used with `PreferencesModal`. | ||
|
||
## Example | ||
|
||
See the `PreferencesModal` readme for usage info. | ||
|
||
|
||
## Props | ||
|
||
### title | ||
|
||
The title of the section | ||
|
||
- Type: `String` | ||
- Required: Yes. | ||
|
||
### description | ||
|
||
The description for the section. | ||
|
||
- Type: `String` | ||
- Required: No. | ||
|
||
### children | ||
|
||
Components to be rendered as content. | ||
|
||
- Type: `Element` | ||
- Required: Yes. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
const Section = ( { description, title, children } ) => ( | ||
<fieldset className="interface-preferences-modal__section"> | ||
<legend> | ||
<h2 className="interface-preferences-modal__section-title"> | ||
{ title } | ||
</h2> | ||
{ description && ( | ||
<p className="interface-preferences-modal__section-description"> | ||
{ description } | ||
</p> | ||
) } | ||
</legend> | ||
{ children } | ||
</fieldset> | ||
); | ||
|
||
export default Section; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
.interface-preferences-modal__section { | ||
margin: 0 0 2.5rem 0; | ||
|
||
&:last-child { | ||
margin: 0; | ||
} | ||
} | ||
|
||
.interface-preferences-modal__section-title { | ||
font-size: 0.9rem; | ||
font-weight: 600; | ||
margin-top: 0; | ||
} | ||
|
||
.interface-preferences-modal__section-description { | ||
margin: -$grid-unit-10 0 $grid-unit-10 0; | ||
font-size: $helptext-font-size; | ||
font-style: normal; | ||
color: $gray-700; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
# PreferencesModalTabs | ||
|
||
`PreferencesModalTabs` creates a tabbed interface meant to be used inside a `PreferencesModal`. Markup differs between small and large viewports; on small the tabs are closed by default. | ||
|
||
## Example | ||
|
||
See the `PreferencesModal` readme for usage info. | ||
## Props | ||
### sections | ||
|
||
Sections to populate the modal with. Takes an array of objects, where each should include `name`, `tablabel` and `content`. | ||
|
||
- Type: `Array` | ||
- Required: Yes. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,156 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { useViewportMatch } from '@wordpress/compose'; | ||
import { | ||
__experimentalNavigatorProvider as NavigatorProvider, | ||
__experimentalNavigatorScreen as NavigatorScreen, | ||
__experimentalNavigatorButton as NavigatorButton, | ||
__experimentalNavigatorBackButton as NavigatorBackButton, | ||
__experimentalItemGroup as ItemGroup, | ||
__experimentalItem as Item, | ||
__experimentalHStack as HStack, | ||
__experimentalText as Text, | ||
__experimentalTruncate as Truncate, | ||
FlexItem, | ||
TabPanel, | ||
Card, | ||
CardHeader, | ||
CardBody, | ||
} from '@wordpress/components'; | ||
import { useMemo, useCallback, useState } from '@wordpress/element'; | ||
import { chevronLeft, chevronRight, Icon } from '@wordpress/icons'; | ||
import { isRTL, __ } from '@wordpress/i18n'; | ||
|
||
const PREFERENCES_MENU = 'preferences-menu'; | ||
|
||
export default function PreferencesModalTabs( { sections } ) { | ||
const isLargeViewport = useViewportMatch( 'medium' ); | ||
|
||
// This is also used to sync the two different rendered components | ||
// between small and large viewports. | ||
const [ activeMenu, setActiveMenu ] = useState( PREFERENCES_MENU ); | ||
/** | ||
* Create helper objects from `sections` for easier data handling. | ||
* `tabs` is used for creating the `TabPanel` and `sectionsContentMap` | ||
* is used for easier access to active tab's content. | ||
*/ | ||
const { tabs, sectionsContentMap } = useMemo( () => { | ||
let mappedTabs = { | ||
tabs: [], | ||
sectionsContentMap: {}, | ||
}; | ||
if ( sections.length ) { | ||
mappedTabs = sections.reduce( | ||
( accumulator, { name, tabLabel: title, content } ) => { | ||
accumulator.tabs.push( { name, title } ); | ||
accumulator.sectionsContentMap[ name ] = content; | ||
return accumulator; | ||
}, | ||
{ tabs: [], sectionsContentMap: {} } | ||
); | ||
} | ||
return mappedTabs; | ||
}, [ sections ] ); | ||
|
||
const getCurrentTab = useCallback( | ||
( tab ) => sectionsContentMap[ tab.name ] || null, | ||
[ sectionsContentMap ] | ||
); | ||
|
||
let modalContent; | ||
// We render different components based on the viewport size. | ||
if ( isLargeViewport ) { | ||
modalContent = ( | ||
<TabPanel | ||
className="interface-preferences__tabs" | ||
tabs={ tabs } | ||
initialTabName={ | ||
activeMenu !== PREFERENCES_MENU ? activeMenu : undefined | ||
} | ||
onSelect={ setActiveMenu } | ||
orientation="vertical" | ||
> | ||
{ getCurrentTab } | ||
</TabPanel> | ||
); | ||
} else { | ||
modalContent = ( | ||
<NavigatorProvider | ||
initialPath="/" | ||
className="interface-preferences__provider" | ||
> | ||
<NavigatorScreen path="/"> | ||
<Card isBorderless size="small"> | ||
<CardBody> | ||
<ItemGroup> | ||
{ tabs.map( ( tab ) => { | ||
return ( | ||
<NavigatorButton | ||
key={ tab.name } | ||
path={ tab.name } | ||
as={ Item } | ||
isAction | ||
> | ||
<HStack justify="space-between"> | ||
<FlexItem> | ||
<Truncate> | ||
{ tab.title } | ||
</Truncate> | ||
</FlexItem> | ||
<FlexItem> | ||
<Icon | ||
icon={ | ||
isRTL() | ||
? chevronLeft | ||
: chevronRight | ||
} | ||
/> | ||
</FlexItem> | ||
</HStack> | ||
</NavigatorButton> | ||
); | ||
} ) } | ||
</ItemGroup> | ||
</CardBody> | ||
</Card> | ||
</NavigatorScreen> | ||
{ sections.length && | ||
sections.map( ( section ) => { | ||
return ( | ||
<NavigatorScreen | ||
key={ `${ section.name }-menu` } | ||
path={ section.name } | ||
> | ||
<Card isBorderless size="large"> | ||
<CardHeader | ||
isBorderless={ false } | ||
justify="left" | ||
size="small" | ||
gap="6" | ||
> | ||
<NavigatorBackButton | ||
icon={ | ||
isRTL() | ||
? chevronRight | ||
: chevronLeft | ||
} | ||
aria-label={ __( | ||
'Navigate to the previous view' | ||
) } | ||
/> | ||
<Text size="16"> | ||
{ section.tabLabel } | ||
</Text> | ||
</CardHeader> | ||
<CardBody>{ section.content }</CardBody> | ||
</Card> | ||
</NavigatorScreen> | ||
); | ||
} ) } | ||
</NavigatorProvider> | ||
); | ||
} | ||
|
||
return modalContent; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
$vertical-tabs-width: 160px; | ||
|
||
.interface-preferences__tabs { | ||
.components-tab-panel__tabs { | ||
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. Ideally we should try to avoid using internal components classnames (as they are not regarded as "public APIs") and to override styles this way, as those styles can easily break in case the component is updated in the future. I understand this may also be caused by a limitation in 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. Thanks for your feedback @ciampo ! These styles were copied verbatim from the original The problem is In more general terms, are there any thoughts around expanding the components API to allow applying styles to child elements of a component? It's quite frequent across the codebase that we resort to the internal classnames for this end. It would be good to find a solution, e.g.:
Let me know what you think! 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.
Thank you for the explanation, I previously had missed this detail (my fault, I skipped the PR's description and went straight to the ping from Riad). I'd say, let's try to see if there's any low-hanging fruit (like this comment) and leave more complicated refactors for another time!
That would be slightly better, but we can probably leave it as-is in this PR. I would be personally questioning why we need to override so many styles for the
Understood — feel free to create an issue for that, and tag @mirka and me.
On this topic, we think that hardcoded classnames and internal DOM structure should not be part of the public APIs of components, as they would severely limit the amount of internal (private) changes that can be made to the components; most internal changes would likely cause breakages for the consumers of such components. I also agree that exposing more props for children classnames is not the correct solution, as it would scale very poorly. Regarding CSS custom properties, it's something that we're going to take a look at soon when we'll start actively working on introducing a lightweight theming API layer. Another approach that we could have is to try and be more "modular" with our components — e.g. expose a cc @mirka 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. |
||
position: absolute; | ||
top: $header-height + $grid-unit-30; | ||
// Aligns button text instead of button box. | ||
left: $grid-unit-20; | ||
width: $vertical-tabs-width; | ||
.components-tab-panel__tabs-item { | ||
border-radius: $radius-block-ui; | ||
font-weight: 400; | ||
&.is-active { | ||
background: $gray-100; | ||
box-shadow: none; | ||
font-weight: 500; | ||
} | ||
&:focus:not(:disabled) { | ||
box-shadow: inset 0 0 0 var(--wp-admin-border-width-focus) var(--wp-admin-theme-color); | ||
} | ||
} | ||
} | ||
.components-tab-panel__tab-content { | ||
padding-left: $grid-unit-30; | ||
margin-left: $vertical-tabs-width; | ||
} | ||
} | ||
|
||
@media (max-width: #{ ($break-medium - 1) }) { | ||
// Keep the navigator component from overflowing the modal content area | ||
// to ensure that sticky position elements stick where intended. | ||
.interface-preferences__provider { | ||
height: 100%; | ||
} | ||
} |
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.
Once #39158 is merged, a next step will be to add a
PreferencesToggleControl
to the preferences package (similar to the wayPreferencesToggleMenuItem
works).I'm not sure what the best way to integrate it with this generic toggle control will be 🤔
The preferences package could use this component, but then that becomes only useful in the preferences modal (and I don't think
preferences
should depend oninterface
).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.
The only reason to have this wrapper around
ToggleControl
is to allow children to be attached to it. If we implement aPreferencesToggleControl
with that capacity, we don't need this component at all.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.
Maybe we should start with this one being
__unstable
and then see how the other toggle component evolves.