Refactor zoom-out iframe scale #59618
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
ajlende
commented
Mar 6, 2024
Comment on lines
+116
to
+120
| computeIFrameScale( | ||
| { width: 1000, scale: 0.45 }, | ||
| { width: 400, scale: 0.9 }, | ||
| contentWidth | ||
| ) |
There was a problem hiding this comment.
I think this reads better enough that you don't need a comment anymore.
Comment on lines
+62
to
+64
| Math.min( p0.y, p1.y ), | ||
| twoPointLinearFn( p0, p1, x ), | ||
| Math.max( p0.y, p1.y ) |
There was a problem hiding this comment.
Order of the points doesn't matter since this has a min and max now.
Comment on lines
+273
to
+274
| // TODO: `marginBottom` doesn't work in Firefox. We need another way to do this. | ||
| iframeDocument.documentElement.style.marginBottom = `${ bottomFrameSize }px`; |
There was a problem hiding this comment.
I found a Firefox bug, but didn't think it was worth fixing here.
Comment on lines
+107
to
+108
| scale = 1, | ||
| frameSize = 0, |
There was a problem hiding this comment.
These are the main things that needed to be restored since this component is exported from the package.
|
Size Change: +30 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Mar 27, 2024
Co-authored-by: ajlende <ajlende@git.wordpress.org> Co-authored-by: scruffian <scruffian@git.wordpress.org>
I think by moving the zoom out mode work to the |
Yes, but I wouldn't say it's as simple as reverting this change. What we're calling Once those changes are made the logic can move to wherever the new view exists. The logic isn't exported from the package so it would be a non-breaking change. But currently the visuals are tied up with the current EDIT: To clarify things a bit more... The solution to getting things working in the post editor is to make the zoom out view work similarly to the mobile/device views—it shouldn't be tied directly to the
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Refactors the changes from #59342 to maintain backwards compatibility with the old
IFramecomponent and simplifies/documents some code.Why?
I noticed that we changed a public API in #59342. The API is
__unstable, but I figure those components would follow the same rules for backwards compatibility as__experimentalsince the addition ofprivateApis.This also removes the need for the iframe to need to know about the
zoom-outmode which I think is slightly better for code organization.How?
Testing Instructions
See that the scaling for zoom-out mode works as it did before.
Testing Instructions for Keyboard
Screenshots or screencast