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

Polish zoom out inserter #66110

Merged
merged 2 commits into from
Oct 15, 2024
Merged

Polish zoom out inserter #66110

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
a95ca72
Hide overflow; add delay to prompt; revise font scale
stokesman Oct 15, 2024
c3c002a
Tweak prompt text transition
stokesman Oct 15, 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
5 changes: 3 additions & 2 deletions packages/block-editor/src/components/block-list/content.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,14 +404,15 @@ _::-webkit-full-page-media, _:future, :root .has-multi-selection .block-editor-b
display: flex;
align-items: center;
justify-content: center;
overflow: hidden;
font-size: $default-font-size;
font-family: $default-font;
color: $black;
font-weight: normal;

.is-zoomed-out & {
// Scale the font size based on the zoom level.
font-size: calc(#{$default-font-size} * ( 2 - var(--wp-block-editor-iframe-zoom-out-scale) ));
// Maintains an absolute font-size by counter-scaling based on the zoom level.
font-size: calc(#{$default-font-size} / var(--wp-block-editor-iframe-zoom-out-scale));
Comment on lines +414 to +415
Copy link
Contributor

Choose a reason for hiding this comment

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

@jasmussen specifically want this to match the default editor font size but scaled for Zoom Out. From what I can see in testing this PR the font size continues to visually match but it would be good to get a confirmation from him.

Copy link
Contributor

Choose a reason for hiding this comment

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

For further context, the goal here isn't so much to match the editor font size, as it is to resemble user interface rather than an element that could be part of the theme itself. I realize that requires some careful math. Ideally the same font, size, appearance applies to this message, as would apply to any other "Drag files here" message.

}

&.is-dragged-over {
Expand Down
14 changes: 8 additions & 6 deletions packages/block-editor/src/components/block-list/zoom-out-separator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,15 @@ export function ZoomOutSeparator( {
<AnimatePresence>
{ isVisible && (
<motion.div
as="button"
layout={ ! isReducedMotion }
Comment on lines -104 to -105
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can’t see why this is currently a button since it doesn’t have interactivity. Besides that if it’s a button I don’t think it should contain a div.

The layout prop doesn’t do anything in this context.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes we've lost this context. We envisaged that when activated (via keyboard or mouse) focus would transfer to the "Search" of the Patterns Inserter. So this is why they were buttons.

I tried that but realised the approach was off so had to revert in #64748.

Given we're at RC for 6.7, I'm not sure we're going to land that feature unless it qualifies as an a11y bug fix. Maybe @jeryj can advise?

initial={ { height: 0 } }
animate={ { height: '120px' } }
animate={ {
// Use a height equal to that of the zoom out frame size.
height: 'calc(1 * var(--wp-block-editor-iframe-zoom-out-frame-size) / var(--wp-block-editor-iframe-zoom-out-scale)',
Copy link
Contributor

Choose a reason for hiding this comment

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

The result seems to be that these drop zones have become smaller. The scale may be correct but I'd recommend a design confidence check from @jasmussen.

} }
exit={ { height: 0 } }
transition={ {
type: 'tween',
duration: 0.2,
duration: isReducedMotion ? 0 : 0.2,
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch 👍

ease: [ 0.6, 0, 0.4, 1 ],
} }
className={ clsx(
Expand All @@ -124,10 +125,11 @@ export function ZoomOutSeparator( {
<motion.div
initial={ { opacity: 0 } }
animate={ { opacity: 1 } }
exit={ { opacity: 0 } }
exit={ { opacity: 0, transition: { delay: -0.125 } } }
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: we should consider a comment here to explain why the delay is necessary without the developer having to go and look up a lot of history to understand.

transition={ {
type: 'tween',
ease: 'linear',
duration: 0.1,
delay: 0.125,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: we should consider a comment here to explain why the delay is necessary without the developer having to go and look up a lot of history to understand.

} }
>
{ __( 'Drop pattern.' ) }
Expand Down
Loading