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.

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

Fix for Popover grid positioning in the FSE #199

Merged
merged 3 commits into from
Jun 27, 2023

Conversation

bmarshall511
Copy link
Contributor

Description of the Change

Fixes the Popover grid positioning in the FSE.

Closes #198

How to test the Change

  1. Go to the FSE & edit a template or page
  2. Add/edit a text component, then click on the insert character button in the toolbar
  3. Popover should appear close to the caret

Changelog Entry

Fixed - Fix for the Popover grid positioning in the FSE

Credits

Props @bmarshall511

Checklist:

@bmarshall511 bmarshall511 added needs:discussion This requires discussion to determine next steps. javascript Pull requests that update Javascript code labels Jun 26, 2023
@bmarshall511 bmarshall511 self-assigned this Jun 26, 2023
@bmarshall511
Copy link
Contributor Author

@fabiankaegy I've implemented what I think is a fairly hacky fix for the Popover grid positioning in the FSE. There's gotta be a better way to select the FSE iframe than what I'm doing here. Tried Googling around for a while with no luck. Do you have any ideas? I'm hoping the blocks api provides something I can use instead on manually getting the iframe via the name attribute. Forgive the dirty code, just wanted to get something up that worked & could be reviewed for suggestions on a better way of handling it.

@fabiankaegy
Copy link
Member

@bmarshall511 I would take a look at the core link format aswell. It appears they are using the contentRef parameter which also gets passed into the edit function. This should be a dom node. Which would allow you to call element.ownerDocument to access the current document element. https://github.com/WordPress/gutenberg/blob/d81637e847cc9474696fc62e2456bb797e09c32f/packages/format-library/src/link/inline.js#L41C2-L41C12

On top of that the core link format use the useAnchor hook with the contentRef passed in to generate the anchor coordinates. Maybe that is worth a look.

@bmarshall511 bmarshall511 marked this pull request as ready for review June 26, 2023 18:41
@bmarshall511 bmarshall511 requested a review from a team as a code owner June 26, 2023 18:41
@bmarshall511 bmarshall511 requested review from Sidsector9 and removed request for a team June 26, 2023 18:41
@bmarshall511
Copy link
Contributor Author

@bmarshall511 I would take a look at the core link format aswell. It appears they are using the contentRef parameter which also gets passed into the edit function. This should be a dom node. Which would allow you to call element.ownerDocument to access the current document element. https://github.com/WordPress/gutenberg/blob/d81637e847cc9474696fc62e2456bb797e09c32f/packages/format-library/src/link/inline.js#L41C2-L41C12

On top of that the core link format use the useAnchor hook with the contentRef passed in to generate the anchor coordinates. Maybe that is worth a look.

@fabiankaegy Thanks! contentRef was the missing piece. Was using the code previously implemented as a guide which was using document to find the caret position. This makes the code much cleaner. It's now working both of FSE and the page editor.

Copy link
Member

@fabiankaegy fabiankaegy left a comment

Choose a reason for hiding this comment

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

These changes look great to me :)

@jeffpaul jeffpaul modified the milestones: 1.1.0, 1.0.7 Jun 27, 2023
@jeffpaul jeffpaul merged commit faee73d into develop Jun 27, 2023
@jeffpaul jeffpaul deleted the fix/fse-grid-positioning-ben branch June 27, 2023 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code needs:discussion This requires discussion to determine next steps.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

FSE grid positioning is off/inconsistent
3 participants