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

JIPT (Just In Place Translation) fix up dependency types #879

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fair-walls-give.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": minor
---

Nest JIPT config options in Perseus dependencies so when its disabled you don't have to pass any supporting functions
6 changes: 4 additions & 2 deletions packages/perseus/src/components/graphie-movables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,14 @@
coord = graphie.unscalePoint(coord);
}
let elem = null;
const {JIPT} = getDependencies();

// If the label is rendered for a locale other than "en", push the label
// element to an array. This array is used to lookup the label element
// and processed with jipt('just in place translation', crowdin specific
// program) to replace the passed in crowdin string with the translated
// string. For "en" locale, the jipt processing is skipped.
if (getDependencies().JIPT.useJIPT) {
if (JIPT.useJIPT) {
elem = graphie.label(
coord,
props.text,
Expand All @@ -139,7 +141,7 @@
props.style,
);

getDependencies().graphieMovablesJiptLabels.addLabel(elem, props.tex);
JIPT.graphieMovablesJiptLabels.addLabel(elem, props.tex);

Check warning on line 144 in packages/perseus/src/components/graphie-movables.ts

View check run for this annotation

Codecov / codecov/patch

packages/perseus/src/components/graphie-movables.ts#L144

Added line #L144 was not covered by tests
} else {
elem = graphie.label(
coord,
Expand Down
5 changes: 1 addition & 4 deletions packages/perseus/src/components/svg-image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -475,10 +475,7 @@
false,
);

getDependencies().svgImageJiptLabels.addLabel(
elem,
labelData.typesetAsMath,
);
JIPT.svgImageJiptLabels.addLabel(elem, labelData.typesetAsMath);

Check warning on line 478 in packages/perseus/src/components/svg-image.tsx

View check run for this annotation

Codecov / codecov/patch

packages/perseus/src/components/svg-image.tsx#L478

Added line #L478 was not covered by tests
} else if (labelData.coordinates) {
// Create labels from the data
// TODO(charlie): Some erroneous labels are being sent down
Expand Down
12 changes: 6 additions & 6 deletions packages/perseus/src/renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -427,13 +427,14 @@
// widgetIds and the child refs of the renderer.
// See: https://phabricator.khanacademy.org/D32420.)
this.widgetIds = [];
const {JIPT} = getDependencies();

if (this.translationIndex != null) {
if (this.translationIndex != null && JIPT.useJIPT) {
// NOTE(jeremy): Since the translationIndex is simply the array
// index of each renderer, we can't remove Renderers from this
// list, rather, we simply null out the entry (which means that
// this array's growth is unbounded until a page reload).
getDependencies().rendererTranslationComponents.removeComponentAtIndex(
JIPT.rendererTranslationComponents.removeComponentAtIndex(

Check warning on line 437 in packages/perseus/src/renderer.tsx

View check run for this annotation

Codecov / codecov/patch

packages/perseus/src/renderer.tsx#L437

Added line #L437 was not covered by tests
this.translationIndex,
);
}
Expand Down Expand Up @@ -1879,11 +1880,10 @@
// before jipt has a chance to replace its contents, so this check
// will keep us from adding the component to the registry a second
// time.
if (!this.translationIndex) {
const {JIPT} = getDependencies();
if (!this.translationIndex && JIPT.useJIPT) {

Check warning on line 1884 in packages/perseus/src/renderer.tsx

View check run for this annotation

Codecov / codecov/patch

packages/perseus/src/renderer.tsx#L1883-L1884

Added lines #L1883 - L1884 were not covered by tests
this.translationIndex =
getDependencies().rendererTranslationComponents.addComponent(
this,
);
JIPT.rendererTranslationComponents.addComponent(this);

Check warning on line 1886 in packages/perseus/src/renderer.tsx

View check run for this annotation

Codecov / codecov/patch

packages/perseus/src/renderer.tsx#L1886

Added line #L1886 was not covered by tests
}

// For articles, we add jipt data to individual paragraphs. For
Expand Down
22 changes: 14 additions & 8 deletions packages/perseus/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,16 @@ export type DomInsertCheckFn = (
jiptString?: string,
) => string | false;

export type JIPT = {
useJIPT: boolean;
};
export type JIPT =
| {
useJIPT: false;
}
| {
useJIPT: true;
graphieMovablesJiptLabels: JiptLabelStore;
svgImageJiptLabels: JiptLabelStore;
rendererTranslationComponents: JiptTranslationComponents;
};

export type JiptLabelStore = {
addLabel: (label?: any, useMath?: any) => void;
Expand Down Expand Up @@ -304,15 +311,14 @@ export type VideoKind = "YOUTUBE_ID" | "READABLE_ID";
// could be used. Aim to shrink the footprint of PerseusDependencies and try to
// use alternative methods where possible.
export type PerseusDependencies = {
// JIPT
JIPT: JIPT;
graphieMovablesJiptLabels: JiptLabelStore;
svgImageJiptLabels: JiptLabelStore;
rendererTranslationComponents: JiptTranslationComponents;

TeX: React.ComponentType<TeXProps>;

//misc
// misc
// Provides a function to transform a relative or absolute URL into a
// request to a static hosting service (think something like an S3 or GCS
// bucket).
staticUrl: StaticUrlFn;
InitialRequestUrl: InitialRequestUrlInterface;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@ describe("graded-group-set", () => {
...testDependencies,
JIPT: {
useJIPT: true,
graphieMovablesJiptLabels: {
addLabel: (label, useMath) => {},
},
svgImageJiptLabels: {
addLabel: (label, useMath) => {},
},
rendererTranslationComponents: {
addComponent: (renderer) => 0,
removeComponentAtIndex: (index) => undefined,
},
},
});
});
Expand Down