Skip to content

Commit 34d5be2

Browse files
cocomarinecocomarine
authored
Only retrieve cached projects for the current locale (#1342)
## Changes ### Locale switching fix - Before: caching wasn't working properly, hence the editor-standalone not behaving consistently and the CCP broken with locale switching (see https://www.figma.com/board/21lOTDiDkQ0cdUNF2fpnZh/Editor-behaviour-on-locale-change?node-id=0-1&p=f&t=osxIb33Az8NX4iqL-0) - Added a locale match check before loading a cached project in `useProject` hook - **This is a temporary fix as it effectively disables caching.** - Both editor-standalone and projects-ui behave in the same way after this change. - Updated existing caching-related tests in `useProject.test` ### Initial load problem fix - Before: when an editor page with locales other than en was first loaded in CCP, it rendered en page rather than the locale page (editor-standalone was fine) - Updated to make sure initial attributes like locale are passed to React by reading them from the DOM in `reactProps()` of `web-component.js` - This is so that the first render uses the host's value even when `attributeChangedCallback` hasn't been run yet
1 parent 32f37a1 commit 34d5be2

5 files changed

Lines changed: 105 additions & 47 deletions

File tree

‎src/containers/WebComponentLoader.jsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ const WebComponentLoader = (props) => {
150150
loadCache,
151151
remixLoadFailed,
152152
initialProject,
153+
locale,
153154
});
154155

155156
useProjectPersistence({

‎src/containers/WebComponentLoader.test.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@ describe("When no user is in state", () => {
257257
initialProject: null,
258258
accessToken: undefined,
259259
loadRemix: false,
260+
locale: "en",
260261
loadCache: true,
261262
remixLoadFailed: false,
262263
reactAppApiEndpoint: "http://localhost:3009",
@@ -363,6 +364,7 @@ describe("When no user is in state", () => {
363364
initialProject: null,
364365
accessToken: undefined,
365366
loadRemix: false,
367+
locale: "en",
366368
loadCache: true,
367369
remixLoadFailed: false,
368370
reactAppApiEndpoint: "http://localhost:3009",
@@ -399,6 +401,7 @@ describe("When no user is in state", () => {
399401
initialProject: null,
400402
accessToken: "my_token",
401403
loadRemix: true,
404+
locale: "en",
402405
loadCache: true,
403406
remixLoadFailed: false,
404407
reactAppApiEndpoint: "http://localhost:3009",
@@ -528,6 +531,7 @@ describe("When user is in state", () => {
528531
initialProject: null,
529532
accessToken: "my_token",
530533
loadRemix: true,
534+
locale: "en",
531535
loadCache: true,
532536
remixLoadFailed: false,
533537
reactAppApiEndpoint: "http://localhost:3009",
@@ -559,6 +563,7 @@ describe("When user is in state", () => {
559563
initialProject: null,
560564
accessToken: "my_token",
561565
loadRemix: false,
566+
locale: "en",
562567
loadCache: true,
563568
remixLoadFailed: false,
564569
reactAppApiEndpoint: "http://localhost:3009",
@@ -640,6 +645,7 @@ describe("When user is in state", () => {
640645
initialProject: null,
641646
accessToken: "my_token",
642647
loadRemix: false,
648+
locale: "en",
643649
loadCache: true,
644650
remixLoadFailed: true,
645651
reactAppApiEndpoint: "http://localhost:3009",

‎src/hooks/useProject.js‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export const useProject = ({
1515
loadRemix = false,
1616
loadCache = true,
1717
remixLoadFailed = false,
18+
locale = null,
1819
}) => {
1920
const loading = useSelector((state) => state.editor.loading);
2021
const isEmbedded = useSelector((state) => state.editor.isEmbedded);
@@ -31,6 +32,7 @@ export const useProject = ({
3132
);
3233
const { i18n } = useTranslation();
3334
const dispatch = useDispatch();
35+
const effectiveLocale = locale ?? i18n.language;
3436

3537
const loadCachedProject = () => {
3638
dispatch(setProject(cachedProject));
@@ -49,7 +51,15 @@ export const useProject = ({
4951
const is_cached_unsaved_project =
5052
!projectIdentifier && cachedProject && !initialProject;
5153

52-
if (loadCache && (is_cached_saved_project || is_cached_unsaved_project)) {
54+
// At the moment this will never match because the cachedProject doesn't have a locale attribute (yet),
55+
// so this will always be false, which effectively disables the whole caching mechanism
56+
const cachedLocaleMatches = cachedProject?.locale === effectiveLocale;
57+
58+
if (
59+
loadCache &&
60+
(is_cached_saved_project || is_cached_unsaved_project) &&
61+
cachedLocaleMatches
62+
) {
5363
loadCachedProject();
5464
return;
5565
}
@@ -59,7 +69,7 @@ export const useProject = ({
5969
syncProject("load")({
6070
reactAppApiEndpoint,
6171
identifier: assetsIdentifier,
62-
locale: i18n.language,
72+
locale: effectiveLocale,
6373
accessToken,
6474
assetsOnly: true,
6575
}),
@@ -72,7 +82,7 @@ export const useProject = ({
7282
syncProject("load")({
7383
reactAppApiEndpoint,
7484
identifier: projectIdentifier,
75-
locale: i18n.language,
85+
locale: effectiveLocale,
7686
accessToken: accessToken,
7787
}),
7888
);
@@ -102,7 +112,7 @@ export const useProject = ({
102112
code,
103113
projectIdentifier,
104114
cachedProject,
105-
i18n.language,
115+
effectiveLocale,
106116
accessToken,
107117
loadRemix,
108118
initialProject,
@@ -134,14 +144,14 @@ export const useProject = ({
134144
syncProject("load")({
135145
reactAppApiEndpoint,
136146
identifier: projectIdentifier,
137-
locale: i18n.language,
147+
locale: effectiveLocale,
138148
accessToken: accessToken,
139149
}),
140150
);
141151

142152
loadDispatched.current = true;
143153
}
144-
}, [projectIdentifier, i18n.language, accessToken, remixLoadFailed]);
154+
}, [projectIdentifier, effectiveLocale, accessToken, remixLoadFailed]);
145155

146156
useEffect(() => {
147157
if (code && loading === "success") {

‎src/hooks/useProject.test.js‎

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,13 @@ const cachedProject = {
2727
project_type: "python",
2828
identifier: "hello-world-project",
2929
components: [],
30+
locale: "fr-FR",
3031
};
3132

3233
const project1 = {
3334
project_type: "python",
3435
identifier: "my-favourite-project",
36+
locale: "es-ES",
3537
};
3638

3739
const accessToken = "my_access_token";
@@ -67,31 +69,39 @@ describe("When not embedded", () => {
6769
expect(setProject).toHaveBeenCalledWith(initialProject);
6870
});
6971

70-
test("If cached project matches identifer uses cached project", () => {
72+
test("If cached project matches identifer and locale, uses cached project", () => {
7173
localStorage.setItem(
7274
cachedProject.identifier,
7375
JSON.stringify(cachedProject),
7476
);
7577
renderHook(
76-
() => useProject({ projectIdentifier: cachedProject.identifier }),
78+
() =>
79+
useProject({
80+
projectIdentifier: cachedProject.identifier,
81+
locale: cachedProject.locale,
82+
}),
7783
{ wrapper },
7884
);
7985
expect(setProject).toHaveBeenCalledWith(cachedProject);
8086
});
8187

82-
test("If cached project matches identifer clears cached project", () => {
88+
test("If cached project matches identifer and locale, clears cached project", () => {
8389
localStorage.setItem(
8490
cachedProject.identifier,
8591
JSON.stringify(cachedProject),
8692
);
8793
renderHook(
88-
() => useProject({ projectIdentifier: cachedProject.identifier }),
94+
() =>
95+
useProject({
96+
projectIdentifier: cachedProject.identifier,
97+
locale: cachedProject.locale,
98+
}),
8999
{ wrapper },
90100
);
91101
expect(localStorage.getItem("project")).toBeNull();
92102
});
93103

94-
test("If cached project does not match identifer does not use cached project", async () => {
104+
test("If cached project does not match identifer, does not use cached project", async () => {
95105
syncProject.mockImplementationOnce(jest.fn((_) => jest.fn()));
96106
localStorage.setItem("project", JSON.stringify(cachedProject));
97107
renderHook(
@@ -106,13 +116,31 @@ describe("When not embedded", () => {
106116
);
107117
});
108118

109-
test("If cached project does not match identifier loads correct uncached project", async () => {
119+
test("If cached project does not match locale, does not use cached project", async () => {
120+
syncProject.mockImplementationOnce(jest.fn((_) => jest.fn()));
121+
localStorage.setItem("project", JSON.stringify(cachedProject));
122+
renderHook(
123+
() =>
124+
useProject({
125+
projectIdentifier: cachedProject.identifier,
126+
locale: "ja-JP",
127+
}),
128+
{ wrapper },
129+
);
130+
expect(syncProject).toHaveBeenCalledWith("load");
131+
await waitFor(() =>
132+
expect(setProject).not.toHaveBeenCalledWith(cachedProject),
133+
);
134+
});
135+
136+
test("If cached project does not match identifier and locale, loads correct uncached project", async () => {
110137
syncProject.mockImplementationOnce(jest.fn((_) => loadProject));
111138
localStorage.setItem("project", JSON.stringify(cachedProject));
112139
renderHook(
113140
() =>
114141
useProject({
115142
projectIdentifier: project1.identifier,
143+
locale: project1.locale,
116144
accessToken,
117145
reactAppApiEndpoint,
118146
}),
@@ -122,14 +150,14 @@ describe("When not embedded", () => {
122150
await waitFor(() =>
123151
expect(loadProject).toHaveBeenCalledWith({
124152
identifier: project1.identifier,
125-
locale: "ja-JP",
153+
locale: project1.locale,
126154
accessToken,
127155
reactAppApiEndpoint,
128156
}),
129157
);
130158
});
131159

132-
test("If loadCache is set to false it loads correct uncached project", async () => {
160+
test("If loadCache is set to false, loads correct uncached project", async () => {
133161
syncProject.mockImplementationOnce(jest.fn((_) => loadProject));
134162
localStorage.setItem("project", JSON.stringify(cachedProject));
135163
renderHook(
@@ -153,7 +181,7 @@ describe("When not embedded", () => {
153181
);
154182
});
155183

156-
test("If no cached project loads uncached project", async () => {
184+
test("If no cached project, loads uncached project", async () => {
157185
syncProject.mockImplementationOnce(jest.fn((_) => loadProject));
158186
renderHook(
159187
() =>
@@ -187,7 +215,7 @@ describe("When not embedded", () => {
187215
await waitFor(() => expect(setProject).not.toHaveBeenCalled());
188216
});
189217

190-
test("If new tab browser preview, uses cached changes", () => {
218+
test("If new tab browser preview and cached locale matches, uses cached changes", () => {
191219
localStorage.setItem("hello-world-project", JSON.stringify(cachedProject));
192220
renderHook(
193221
() =>
@@ -196,6 +224,7 @@ describe("When not embedded", () => {
196224
accessToken,
197225
isEmbedded: true,
198226
isBrowserPreview: true,
227+
locale: "fr-FR",
199228
}),
200229
{ wrapper },
201230
);

‎src/web-component.js‎

Lines changed: 43 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -78,40 +78,42 @@ class WebComponent extends HTMLElement {
7878
];
7979
}
8080

81+
parseAttribute(name, rawValue) {
82+
const boolAttrs = [
83+
"embedded",
84+
"editable_instructions",
85+
"load_remix_disabled",
86+
"output_only",
87+
"output_split_view",
88+
"project_name_editable",
89+
"read_only",
90+
"sense_hat_always_enabled",
91+
"show_save_prompt",
92+
"use_editor_styles",
93+
"with_projectbar",
94+
"with_sidebar",
95+
"load_cache",
96+
];
97+
const jsonAttrs = [
98+
"instructions",
99+
"sidebar_options",
100+
"host_styles",
101+
"output_panels",
102+
];
103+
if (boolAttrs.includes(name)) return rawValue !== "false";
104+
if (jsonAttrs.includes(name))
105+
return rawValue ? JSON.parse(rawValue) : undefined;
106+
return rawValue;
107+
}
108+
81109
attributeChangedCallback(name, _oldVal, newVal) {
82-
let value;
83-
84-
if (
85-
[
86-
"embedded",
87-
"editable_instructions",
88-
"load_remix_disabled",
89-
"output_only",
90-
"output_split_view",
91-
"project_name_editable",
92-
"read_only",
93-
"sense_hat_always_enabled",
94-
"show_save_prompt",
95-
"use_editor_styles",
96-
"with_projectbar",
97-
"with_sidebar",
98-
"load_cache",
99-
].includes(name)
100-
) {
101-
value = newVal !== "false";
102-
} else if (
103-
[
104-
"instructions",
105-
"sidebar_options",
106-
"host_styles",
107-
"output_panels",
108-
].includes(name)
109-
) {
110-
value = JSON.parse(newVal);
110+
const key = camelCase(name);
111+
if (newVal === null) {
112+
delete this.componentAttributes[key];
111113
} else {
112-
value = newVal;
114+
const value = this.parseAttribute(name, newVal);
115+
this.componentAttributes[key] = value;
113116
}
114-
this.componentAttributes[camelCase(name)] = value;
115117
this.mountReactApp();
116118
}
117119

@@ -166,7 +168,17 @@ class WebComponent extends HTMLElement {
166168
}
167169

168170
reactProps() {
171+
const observed = this.constructor.observedAttributes || [];
172+
const fromDom = {};
173+
for (const name of observed) {
174+
if (this.hasAttribute(name)) {
175+
const raw = this.getAttribute(name);
176+
fromDom[camelCase(name)] = this.parseAttribute(name, raw);
177+
}
178+
}
179+
169180
return {
181+
...fromDom,
170182
...this.componentAttributes,
171183
...this.componentProperties,
172184
};

0 commit comments

Comments
 (0)