Skip to content

Commit

Permalink
Dom sampler measures grid placeholders to store grid cell global fram…
Browse files Browse the repository at this point in the history
…es (#6410)

**Problem:**
Setting `justifyContent` or `alignContent` etc properties is not
reflected in the calculated grid cell bounds (and that ruins most grid
interactions and controls).

**Fix:**
So it turned out that calculating the grid cell bounds from the
computedStyle is just too complicated and error prone. Calculating the
grid cell width/height is easy, but to get the position we would need to
reimplement some of the grid layouting algorithm of the browser.
The solution is to go back to measure the placeholders of the grid
control in the DOM, but let's do it in the dom sampler, and save the
measurements in the metadata. This way we can keep the advantages of the
domquery -> metadata refactor (because all strategies and controls still
use the metadata to get the cell bounds), but we can still rely on the
precise dom measurements. Using the dom api is fine, because we keep it
inside the dom sampler which does dom measurements anyway.

This solution needs 2 extra hacks to work properly:

- sometimes the grid controls are not on the screen when an interaction
starts. In these cases the startingMetadata does not contain the grid
cell bounds. As a workaround I wrote a helper which gets the bounds from
either the startingMetadata or the latestMetadata, and stores it in the
custom strategy state. So in these grid strategies we don't really use
the startingMetadata, but the first metadata which already contains the
grid bounds.

- the grid cells are measured on the grid controls, but the controls
render later than then canvas. First canvas store is flushed, then the
dom sampler runs, then the editor store is flushed and the controls are
rendered. This order is important, because the controls rely on the
metadata produced by the dom sampler. This means that after certain
changes (e.g. a zoom in) the grid controls are outdated when the dom
sampler runs, and we can't measure the correct cell bounds. To
circumvent this, I added an observer to the grid controls, which
triggers another dom sampling after a grid control change.

**Commit Details:**

- Introduced new field `gridCellGlobalFrames` in
`specialSizeMeasurements`
- Filling in `gridCellGlobalFrames` in `getSpecialMeasurements` by
finding the grid placeholders in the canvas control.
- Removed `getGlobalFramesOfGridCells`, because it would just need to
return `specialSizeMeasurements.gridCellGlobalFrames`
- Introduced `getMetadataWithGridCellBounds` which gets the metadata
either from startingMetadata or latestMetadada, and chooses the one with
grid cell bounds in it (if possible)
- Added new observer to `DomWalkerMutableState` called
`gridControlObserver`
- Added a parameter to `runDOMWalker` action which makes it possible to
restrict the dom sampler run to a given array of element paths
(necessary so we can trigger a dom walker run only on the grid).

**Manual Tests:**
I hereby swear that:

- [x] I opened a hydrogen project and it loaded
- [x] I could navigate to various routes in Preview mode
  • Loading branch information
gbalint authored Oct 2, 2024
1 parent 9dc2fa3 commit 9d2f887
Show file tree
Hide file tree
Showing 31 changed files with 953 additions and 403 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ Object {
"gap": null,
"globalContentBoxForChildren": null,
"globalFrameWithTextContent": null,
"gridCellGlobalFrames": null,
"hasPositionOffset": false,
"hasTransform": false,
"htmlElementName": "div",
Expand Down Expand Up @@ -358,6 +359,7 @@ Object {
"gap": null,
"globalContentBoxForChildren": null,
"globalFrameWithTextContent": null,
"gridCellGlobalFrames": null,
"hasPositionOffset": false,
"hasTransform": false,
"htmlElementName": "div",
Expand Down
Loading

0 comments on commit 9d2f887

Please sign in to comment.