Skip to content

Commit

Permalink
[Graphie] Adjust Scaling of Text to Render Properly in Mobile (#1540)
Browse files Browse the repository at this point in the history
## Summary:
When a graphie image is displayed in a scaled-down environment (i.e. mobile viewport), SVG elements were scaled properly, but text was not. The issue was due to scaling not being applied to the text. This change adds in the calculations to properly scale and position text that exists in the image.

Issue: LEMS-2022

## Test plan:

1. Obtain a Graphie image.
   * From Graphie editor:
      * Open a Graphie editor for an existing image.
      * Click on the "Convert to image" button.
      * Wait for the image web address to be generated.
   * Or, just use this example image: <br/> web+graphie://ka-perseus-graphie.s3.amazonaws.com/2c7f2619eec00ab1992b4a491a0359542012b286.
2. Open an [editor page](https://www.khanacademy.org/devadmin/content/articles/test-article-for-the-test-course/xf02c39bb55c6da4a) (Storybook editor pages aren't working properly with Graphie images at the time of this PR).
3. Add an image widget.
4. Paste the image URL into the available entry field.
5. Compare the image preview using the available Desktop and Mobile toggles.
6. The text elements (i.e. point labels) should be scaled with the image (lines, etc.) and placed correctly.

## Affected behavior
### Before - bad text
![Bad Text](https://github.com/user-attachments/assets/009bae41-8770-479a-8621-e22300e41fe5)

### After - good text
![Good Text](https://github.com/user-attachments/assets/2289887d-f41c-4035-9ec4-855162081df3)

Author: mark-fitzgerald

Reviewers: nishasy, jeremywiebe, mark-fitzgerald, catandthemachines

Required Reviewers:

Approved By: nishasy, jeremywiebe, catandthemachines

Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1540
  • Loading branch information
mark-fitzgerald authored Sep 10, 2024
1 parent 71715af commit 08068dc
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .changeset/hungry-bobcats-smoke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Resolve improperly scaled text/labels in Graphie images when viewed in mobile (constrained viewports)
12 changes: 11 additions & 1 deletion packages/perseus/src/components/svg-image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -485,12 +485,22 @@ class SvgImage extends React.Component<Props, State> {
// without coordinates. They don't seem to have any content, so
// it seems fine to just ignore them (rather than error), but
// we should figure out why this is happening.

// 'styling' - When a default scale (1) is used,
// setting the font size to 100% is redundant.
// Also, setting the font size to 100% can counteract other scale-based styling.
// Therefore, when the scale is not applicable,
// we set the value to 'null' so that no additional styling will be set.
const styling =
this.props.scale !== 1
? {"font-size": 100 * this.props.scale + "%"}
: null;
const label = graphie.label(
labelData.coordinates,
labelData.content,
labelData.alignment,
labelData.typesetAsMath,
{"font-size": 100 * this.props.scale + "%"},
styling,
);

// Convert absolute positioning css from pixels to percentages
Expand Down
36 changes: 32 additions & 4 deletions packages/perseus/src/util/graphie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1675,11 +1675,39 @@ const setLabelMargins = function (span: HTMLElement, size: Coord): void {
marginTop: -height / 2 - y * scale,
});
} else {
const $container = $span.closest(".svg-image");
// Ensuring that the line-height of the text doesn't throw off placement of the text element.
// Inherited line-height values can really mess up placement.
$container.css("line-height", "normal");

// The 'max-width' value is the same as the expected (100% scale) width of the graphie.
// We are using jQuery to get this information since we don't have a way to pass it to this function.
const expectedWidth = $container.css("max-width") ?? "0px";
// We can calculate the true scale of the graphie by taking actual width and dividing by the expected width.
// NOTE: Using 'slice' to remove the "px" unit from the end of the expected width value.
const scale =
(($container.width() ?? 0) /
parseInt(expectedWidth.replace(/px$/, ""))) *
100;

// Any padding needs to be scaled accordingly.
const currentPadding = $span.css("padding") ?? "0px";
const newPadding =
Math.round(parseInt(currentPadding.slice(0, -2)) * scale) / 100;

// "multipliers" basically move the position of the text by using 1, 0, -1
// 'margin-left' and 'margin-top' are used to position the text.
// Margin and font size need to be scaled accordingly.
const multipliers = labelDirections[direction || "center"];
$span.css({
marginLeft: Math.round(width * multipliers[0]),
marginTop: Math.round(height * multipliers[1]),
});
const styling = {
marginLeft: Math.round(width * multipliers[0] * scale) / 100,
marginTop: Math.round(height * multipliers[1] * scale) / 100,
padding: `${newPadding}px`,
};
if (scale !== 1) {
styling["fontSize"] = `${Math.round(scale * 100) / 100}%`;
}
$span.css(styling);
}
};

Expand Down

0 comments on commit 08068dc

Please sign in to comment.