Skip to content

Commit

Permalink
Guard against matchMedia access during initial render (#1459)
Browse files Browse the repository at this point in the history
## Summary:
This PR makes sure that we don't call `matchMedia` during the initial render. This is because `matchMedia` is not available during the initial render, and calling it will throw an error.

Issue: FEI-5743

## Test plan:
1. Put up this PR and await the pre-release packages
2. Update webapp with the pre-release packages
3. Put up a webapp PR and check that the ZND is working on URLs that were previously erroring

Author: somewhatabstract

Reviewers: somewhatabstract, jeresig, jeremywiebe

Required Reviewers:

Approved By: jeresig, jeremywiebe

Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check builds for changes in size (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: #1459
  • Loading branch information
somewhatabstract authored Jul 30, 2024
1 parent ca31afb commit be40d77
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changeset/quiet-moose-arrive.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Guard against executing matchMedia during initial render
15 changes: 12 additions & 3 deletions packages/perseus/src/widgets/explanation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,15 @@ class Explanation extends React.Component<Props, State> {
state: State = {
expanded: false,
};
_mounted: boolean = false;

componentDidMount() {
this._mounted = true;
}

componentWillUnmount() {
this._mounted = false;
}

change: (arg1: any, arg2: any, arg3: any) => any = (...args) => {
return Changeable.change.apply(this, args);
Expand All @@ -85,9 +94,9 @@ class Explanation extends React.Component<Props, State> {

const caretIcon = this.state.expanded ? caretUp : caretDown;

const allowTransition = mediaQueryIsMatched(
"(prefers-reduced-motion: no-preference)",
);
const allowTransition =
this._mounted &&
mediaQueryIsMatched("(prefers-reduced-motion: no-preference)");

// Special styling is needed to fit the button in a block of text without throwing off the line spacing.
// While the button is not normally included in a block of text, it needs to be able to accommodate such a case.
Expand Down
27 changes: 18 additions & 9 deletions packages/perseus/src/widgets/label-image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export class LabelImage extends React.Component<

// The rendered markers on the question image for labeling.
_markers: Array<Marker | null | undefined>;
_mounted: boolean = false;

static gradeMarker(marker: InteractiveMarkerType): InteractiveMarkerScore {
const score = {
Expand Down Expand Up @@ -375,6 +376,14 @@ export class LabelImage extends React.Component<
};
}

componentDidMount() {
this._mounted = true;
}

componentWillUnmount() {
this._mounted = false;
}

simpleValidate(rubric: LabelImageProps): PerseusScore {
return LabelImage.validate(this.getUserInput(), rubric);
}
Expand Down Expand Up @@ -503,17 +512,17 @@ export class LabelImage extends React.Component<

const {activeMarkerIndex, markersInteracted} = this.state;

// Render all markers for widget.
return markers.map((marker, index): React.ReactElement => {
// Determine whether page is rendered in a narrow browser window.
const isNarrowPage = window.matchMedia(
mediaQueries.xsOrSmaller.replace("@media ", ""),
).matches;
// Determine whether page is rendered in a narrow browser window.
const isNarrowPage =
this._mounted &&
window.matchMedia(mediaQueries.xsOrSmaller.replace("@media ", ""))
.matches;

// Determine whether the image is wider than it is tall.
const isWideImage =
this.props.imageWidth / 2 > this.props.imageHeight;
// Determine whether the image is wider than it is tall.
const isWideImage = this.props.imageWidth / 2 > this.props.imageHeight;

// Render all markers for widget.
return markers.map((marker, index): React.ReactElement => {
let side: "bottom" | "left" | "right" | "top";
let markerPosition;
// Position popup closest to the center, preferring it renders
Expand Down
15 changes: 14 additions & 1 deletion packages/perseus/src/widgets/label-image/marker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ type Props = InteractiveMarkerType & {
};

function shouldReduceMotion(): boolean {
// We cannot use matchMedia during SSR.
if (typeof window.matchMedia !== "function") {
return true;
}
const mediaQuery = window.matchMedia("(prefers-reduced-motion: reduce)");
return !mediaQuery || mediaQuery.matches;
}
Expand All @@ -49,13 +53,22 @@ export default class Marker extends React.Component<Props> {

// The marker icon element.
_icon: HTMLElement | null | undefined;
_mounted: boolean = false;

static defaultProps: {
selected: ReadonlyArray<any>;
} = {
selected: [],
};

componentDidMount() {
this._mounted = true;
}

componentWillUnmount() {
this._mounted = false;
}

renderIcon() {
const {selected, showCorrectness, showSelected, showPulsate} =
this.props;
Expand Down Expand Up @@ -106,7 +119,7 @@ export default class Marker extends React.Component<Props> {
} else if (showPulsate) {
iconStyles = [
styles.markerPulsateBase,
shouldReduceMotion()
this._mounted && shouldReduceMotion()
? showPulsate && styles.markerUnfilledPulsateOnce
: showPulsate && styles.markerUnfilledPulsateInfinite,
];
Expand Down

0 comments on commit be40d77

Please sign in to comment.