-
Notifications
You must be signed in to change notification settings - Fork 98
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
Handle the site_icon_maskable
setting within a full-site-editing context
#919
base: develop
Are you sure you want to change the base?
Changes from all commits
dabe026
a40c268
6469963
0649bca
4737081
bf1a0a5
96cf3da
946c52e
d203583
2e12399
d30ac15
37cf884
dd03b79
1446604
812e59a
81d2860
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 |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
/phpcs.xml | ||
/.phpcs.xml | ||
*.zip | ||
/wp-includes/js/dist/* | ||
/wp-includes/js/workbox* | ||
/wiki | ||
.vscode | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
/** | ||
* The 'icon maskable' control allows to set the 'site_icon_maskable' option. | ||
* | ||
* It also provides a small preview of the image used as Site-Logo, | ||
* using a cropping-preview to illustrate the safe space an logo needs | ||
* to be an adaptive image. | ||
*/ | ||
import MaskableControls from './maskable-icon-controls'; | ||
|
||
/** | ||
* The compose package is a collection | ||
* of handy Hooks and Higher Order Components (HOCs) | ||
* you can use to wrap your WordPress components | ||
* and provide some basic features like: state, instance id, pure… | ||
* | ||
* @see https://developer.wordpress.org/block-editor/reference-guides/packages/packages-compose/ | ||
*/ | ||
import { createHigherOrderComponent } from '@wordpress/compose'; | ||
|
||
const withMaskableControls = createHigherOrderComponent((BlockEdit) => { | ||
return (props) => { | ||
if (props.name !== 'core/site-logo') { | ||
return <BlockEdit {...props} />; | ||
} | ||
|
||
return ( | ||
<> | ||
<BlockEdit {...props} /> | ||
<MaskableControls /> | ||
</> | ||
); | ||
}; | ||
}, 'withMaskableControls'); | ||
|
||
/** | ||
* To modify the behavior of existing blocks, | ||
* WordPress exposes several APIs. | ||
* | ||
* @see https://developer.wordpress.org/block-editor/reference-guides/filters/block-filters/ | ||
*/ | ||
wp.hooks.addFilter( | ||
'editor.BlockEdit', | ||
'pwa/with-maskable-icon-controls', | ||
withMaskableControls | ||
); |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,115 @@ | ||||||
/** | ||||||
* Retrieves the translation of text. | ||||||
* | ||||||
* @see https://developer.wordpress.org/block-editor/packages/packages-i18n/ | ||||||
*/ | ||||||
import { __ } from '@wordpress/i18n'; | ||||||
|
||||||
/** | ||||||
* This module allows you to create and use standalone block editors. ;) | ||||||
* | ||||||
* @see https://developer.wordpress.org/block-editor/reference-guides/packages/packages-block-editor/ | ||||||
*/ | ||||||
import { InspectorControls } from '@wordpress/block-editor'; | ||||||
|
||||||
/** | ||||||
* This package includes a library of generic WordPress components | ||||||
* to be used for creating common UI elements shared between screens | ||||||
* and features of the WordPress dashboard. | ||||||
* | ||||||
* @see https://developer.wordpress.org/block-editor/reference-guides/packages/packages-components/ | ||||||
*/ | ||||||
import { | ||||||
Flex, | ||||||
FlexBlock, | ||||||
FlexItem, | ||||||
PanelBody, | ||||||
ToggleControl, | ||||||
} from '@wordpress/components'; | ||||||
|
||||||
/** | ||||||
* Core Data is a data module intended to | ||||||
* simplify access to and manipulation | ||||||
* of core WordPress entities. | ||||||
* | ||||||
* @see https://developer.wordpress.org/block-editor/reference-guides/packages/packages-core-data/ | ||||||
*/ | ||||||
import { store as coreStore, useEntityProp } from '@wordpress/core-data'; | ||||||
|
||||||
/** | ||||||
* WordPress’ data module serves as a hub | ||||||
* to manage application state | ||||||
* for both plugins and WordPress itself. | ||||||
* | ||||||
* @see https://developer.wordpress.org/block-editor/reference-guides/packages/packages-data/ | ||||||
*/ | ||||||
import { useSelect } from '@wordpress/data'; | ||||||
|
||||||
export default function MaskableControls() { | ||||||
const [siteIconMaskable, setSiteIconMaskable] = useEntityProp( | ||||||
'root', | ||||||
'site', | ||||||
'site_icon_maskable' | ||||||
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. 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. This is unfortunately a known bug in gutenberg, I'm going to ref. the issue here, when I'll find it again ;) |
||||||
); | ||||||
|
||||||
// mainly borrowed 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. From what? 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.
Suggested change
Unfortunately there is no nice docs-page to reference to, propably because this component is part of the Edit Site components, which are described in its own README as
|
||||||
const { isRequestingSiteIcon, siteIconUrl } = useSelect((select) => { | ||||||
const { getEntityRecord, isResolving } = select(coreStore); | ||||||
const siteData = | ||||||
getEntityRecord('root', '__unstableBase', undefined) || {}; | ||||||
|
||||||
return { | ||||||
isRequestingSiteIcon: isResolving('getEntityRecord', [ | ||||||
'root', | ||||||
'__unstableBase', | ||||||
undefined, | ||||||
]), | ||||||
siteIconUrl: siteData.site_icon_url, | ||||||
}; | ||||||
}, []); | ||||||
|
||||||
if (isRequestingSiteIcon) { | ||||||
return null; | ||||||
} | ||||||
|
||||||
const siteIconStyle = { | ||||||
clipPath: siteIconMaskable ? 'inset(10% round 50%)' : '', | ||||||
width: '64px', | ||||||
}; | ||||||
|
||||||
let siteIcon = <div style={siteIconStyle} />; | ||||||
|
||||||
if (siteIconUrl) { | ||||||
siteIcon = ( | ||||||
<img | ||||||
alt={__('Site Icon')} | ||||||
className="components-site-icon" | ||||||
src={siteIconUrl} | ||||||
width={64} | ||||||
height={64} | ||||||
style={siteIconStyle} | ||||||
/> | ||||||
); | ||||||
} | ||||||
Comment on lines
+82
to
+93
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 tried selecting a non-square image as the Site Logo and this is resulting in an unexpected result for the maskable icon: Three things aren't quite right here:
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. OMG. But you are totally right on all of your points. I'll try to find some solutions. |
||||||
|
||||||
return ( | ||||||
<InspectorControls> | ||||||
<PanelBody> | ||||||
<Flex align="start"> | ||||||
<FlexBlock> | ||||||
<ToggleControl | ||||||
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. When I toggle this on the first tine I get a React warning:
Perhaps this is because it is missing a default |
||||||
label={__('Maskable icon', 'pwa')} | ||||||
help={__( | ||||||
'Maskable icons let your Progressive Web App use adaptive icons. If you supply a maskable icon, your icon can fill up the entire shape as an app- or homescreen-icon and will look great on all devices.', | ||||||
'pwa' | ||||||
)} | ||||||
onChange={setSiteIconMaskable} | ||||||
checked={siteIconMaskable} | ||||||
/> | ||||||
</FlexBlock> | ||||||
<FlexItem>{siteIcon}</FlexItem> | ||||||
</Flex> | ||||||
</PanelBody> | ||||||
</InspectorControls> | ||||||
); | ||||||
} |
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.
This is a redundant check with
_pwa_print_build_needed_notice
, right? So is it needed?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.
You are right, that is a redundant check, but leaving it out will result in fatal error with the following
require
statement on the next line. Or am I wrong?Additionally, because this is called inside the block-editor, where the normal
admin_notice
s will not be visible, this should prevent errors while trying to enque a non-existent file. To help the user mentioning, that the plugin was not built properly, we could introduce one of the newercreateWarningNotice
for the block-editor. But this should be new issue, I think.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.
If you see the change you made in
pwa.php
:pwa-wp/pwa.php
Lines 100 to 106 in 81d2860
Note that in the bootstrap, it does a
return
to prevent the rest of the PWA functionality from being loaded. So if the file doesn't exist, then this conditional won't ever be reached.