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

Post editor: Fix "typewriter" spacing style application #65885

Merged
merged 2 commits into from
Oct 8, 2024
Merged

Post editor: Fix "typewriter" spacing style application #65885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
26ba629
Add parameter to enable/disable hook
stokesman Sep 3, 2024
cd45eae
Separate concerns of useEditorStyles and usePaddingAppender
stokesman Sep 29, 2024
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
53 changes: 21 additions & 32 deletions packages/edit-post/src/components/layout/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,27 +84,17 @@ const DESIGN_POST_TYPES = [
'wp_navigation',
];

function useEditorStyles() {
const {
hasThemeStyleSupport,
editorSettings,
isZoomedOutView,
renderingMode,
postType,
} = useSelect( ( select ) => {
const { __unstableGetEditorMode } = select( blockEditorStore );
const { getCurrentPostType, getRenderingMode } = select( editorStore );
const _postType = getCurrentPostType();
function useEditorStyles( ...additionalStyles ) {
const { hasThemeStyleSupport, editorSettings } = useSelect( ( select ) => {
return {
hasThemeStyleSupport:
select( editPostStore ).isFeatureActive( 'themeStyles' ),
editorSettings: select( editorStore ).getEditorSettings(),
isZoomedOutView: __unstableGetEditorMode() === 'zoom-out',
renderingMode: getRenderingMode(),
postType: _postType,
};
}, [] );

const addedStyles = additionalStyles.join( '\n' );
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Currently there is only one style for the padding appender, so no line break will occur, but is the line break itself even necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it’s not necessary. Passing something as a separator is required to avoid the default comma. I suppose I opted for the line break as a formatting nicety despite the fact that it’d be a rare case that anyone is actually looking at the concatenated styles (and that as of yet there’s not more than one style so it’s moot for now). Here’s an example of what it looks like inspecting the element if there were more than one style.

empty string separator:
image

\n seperator:
image

I don’t mind making it an empty string or space if you’d opine it should be.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have any strong opinions about it, so I think it's fine to merge it as is 👍


// Compute the default styles.
return useMemo( () => {
const presetStyles =
Expand Down Expand Up @@ -141,19 +131,8 @@ function useEditorStyles() {
? editorSettings.styles ?? []
: defaultEditorStyles;

// Add a space for the typewriter effect. When typing in the last block,
// there needs to be room to scroll up.
if (
! isZoomedOutView &&
renderingMode === 'post-only' &&
! DESIGN_POST_TYPES.includes( postType )
) {
return [
...baseStyles,
{
css: ':root :where(.editor-styles-wrapper)::after {content: ""; display: block; height: 40vh;}',
},
];
if ( addedStyles ) {
return [ ...baseStyles, { css: addedStyles } ];
}

return baseStyles;
Expand All @@ -162,7 +141,7 @@ function useEditorStyles() {
editorSettings.disableLayoutStyles,
editorSettings.styles,
hasThemeStyleSupport,
postType,
addedStyles,
] );
}

Expand Down Expand Up @@ -394,7 +373,6 @@ function Layout( {
} ) {
useCommands();
useEditPostCommands();
const paddingAppenderRef = usePaddingAppender();
const shouldIframe = useShouldIframe();
const { createErrorNotice } = useDispatch( noticesStore );
const {
Expand All @@ -418,6 +396,7 @@ function Layout( {
hasHistory,
isWelcomeGuideVisible,
templateId,
enablePaddingAppender,
} = useSelect(
( select ) => {
const { get } = select( preferencesStore );
Expand All @@ -433,9 +412,12 @@ function Layout( {
kind: 'postType',
name: 'wp_template',
} );
const { __unstableGetEditorMode } = select( blockEditorStore );
const { getEditorMode, getRenderingMode } = select( editorStore );
const isRenderingPostOnly = getRenderingMode() === 'post-only';

return {
mode: select( editorStore ).getEditorMode(),
mode: getEditorMode(),
isFullscreenActive:
select( editPostStore ).isFeatureActive( 'fullscreenMode' ),
hasActiveMetaboxes: select( editPostStore ).hasMetaBoxes(),
Expand All @@ -445,7 +427,7 @@ function Layout( {
isDistractionFree: get( 'core', 'distractionFree' ),
showMetaBoxes:
! DESIGN_POST_TYPES.includes( currentPostType ) &&
select( editorStore ).getRenderingMode() === 'post-only',
isRenderingPostOnly,
isWelcomeGuideVisible: isFeatureActive( 'welcomeGuide' ),
templateId:
supportsTemplateMode &&
Expand All @@ -454,10 +436,17 @@ function Layout( {
! isEditingTemplate
? getEditedPostTemplateId()
: null,
enablePaddingAppender:
__unstableGetEditorMode() !== 'zoom-out' &&
isRenderingPostOnly &&
! DESIGN_POST_TYPES.includes( currentPostType ),
};
},
[ currentPostType, isEditingTemplate, settings.supportsTemplateMode ]
);
const [ paddingAppenderRef, paddingStyle ] = usePaddingAppender(
enablePaddingAppender
);

// Set the right context for the command palette
const commandContext = hasBlockSelected
Expand All @@ -473,7 +462,7 @@ function Layout( {
} ),
[ settings, onNavigateToEntityRecord, onNavigateToPreviousEntityRecord ]
);
const styles = useEditorStyles();
const styles = useEditorStyles( paddingStyle );

// We need to add the show-icon-labels class to the body element so it is applied to modals.
if ( showIconLabels ) {
Expand Down
22 changes: 8 additions & 14 deletions packages/edit-post/src/components/layout/use-padding-appender.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,20 @@ import { useRefEffect } from '@wordpress/compose';
import { store as blockEditorStore } from '@wordpress/block-editor';
import { isUnmodifiedDefaultBlock } from '@wordpress/blocks';

export function usePaddingAppender() {
// Ruleset to add space for the typewriter effect. When typing in the last
// block, there needs to be room to scroll up.
const CSS =
':root :where(.editor-styles-wrapper)::after {content: ""; display: block; height: 40vh;}';

export function usePaddingAppender( enabled ) {
const registry = useRegistry();
return useRefEffect(
const effect = useRefEffect(
( node ) => {
function onMouseDown( event ) {
if ( event.target !== node ) {
return;
}

const { ownerDocument } = node;
const { defaultView } = ownerDocument;

const pseudoHeight = defaultView.parseInt(
defaultView.getComputedStyle( node, ':after' ).height,
10
);

if ( ! pseudoHeight ) {
return;
}

// Only handle clicks under the last child.
const lastChild = node.lastElementChild;
if ( ! lastChild ) {
Expand Down Expand Up @@ -64,4 +57,5 @@ export function usePaddingAppender() {
},
[ registry ]
);
return enabled ? [ effect, CSS ] : [];
}
Loading