Skip to content

Commit 7ea9e57

Browse files
authored
Fallback when sidebar element does not exist | Scratch editor default behaviour collapsed (#1361)
It was reported: - Clicking the chevron icons causes a "glitchy" visual effect. - A strange grey line appears around the border of the sidebar during or after the interaction. - The chevron icon itself appears to change shape or distort when clicked. After investigation, it was found out: - It normally defaults to instructions, file. - it normally requires to be expanded but not in scratch-editor - These two situations makes it feel glitchy ### Solution - Let still use these as default, but has as fallback the first existing item (both from the top and bottom part) - Let the editor now that in scratch we need to initiate collapsed - Addded test to cover all of these cases https://github.com/user-attachments/assets/bfac2af7-264b-4450-915f-5ab5484f6bf3
1 parent dc3af0a commit 7ea9e57

5 files changed

Lines changed: 86 additions & 12 deletions

File tree

src/components/Menus/Sidebar/Sidebar.jsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ const Sidebar = ({ options = [], plugins = [], allowMobileView = true }) => {
162162
const optionDict = menuOptions.find((menuOption) => {
163163
return menuOption.name === option;
164164
});
165+
const activeOption = optionDict ? option : null;
165166

166167
const CustomSidebarPanel =
167168
optionDict && optionDict.panel ? optionDict.panel : () => {};
@@ -175,12 +176,12 @@ const Sidebar = ({ options = [], plugins = [], allowMobileView = true }) => {
175176
>
176177
<SidebarBar
177178
menuOptions={menuOptions}
178-
option={option}
179+
option={activeOption}
179180
toggleOption={toggleOption}
180181
instructions={instructionsSteps}
181182
allowMobileView={allowMobileView}
182183
/>
183-
{option && <CustomSidebarPanel isMobile={isMobile} />}
184+
{activeOption && <CustomSidebarPanel isMobile={isMobile} />}
184185
</div>
185186
);
186187
};

src/components/Menus/Sidebar/Sidebar.test.js

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,13 @@ let images = [
1616
];
1717

1818
const options = ["file", "images", "instructions", "info"];
19+
const optionsWithDownload = [
20+
"file",
21+
"images",
22+
"instructions",
23+
"download",
24+
"info",
25+
];
1926

2027
describe("When project has images", () => {
2128
describe("and no instructions", () => {
@@ -416,6 +423,9 @@ describe("When the project type is code_editor_scratch", () => {
416423
beforeEach(() => {
417424
const mockStore = configureStore([]);
418425
const initialState = {
426+
auth: {
427+
user: null,
428+
},
419429
editor: {
420430
project: {
421431
components: [],
@@ -430,7 +440,7 @@ describe("When the project type is code_editor_scratch", () => {
430440
render(
431441
<Provider store={store}>
432442
<div id="app">
433-
<Sidebar options={options} />
443+
<Sidebar options={optionsWithDownload} />
434444
</div>
435445
</Provider>,
436446
);
@@ -443,4 +453,14 @@ describe("When the project type is code_editor_scratch", () => {
443453
test("Shows the info icon", () => {
444454
expect(screen.queryByTitle("sidebar.information")).toBeInTheDocument();
445455
});
456+
457+
test("Starts in closed chevron state for scratch", () => {
458+
expect(screen.queryByTitle("sidebar.expand")).toBeInTheDocument();
459+
});
460+
461+
test("Clicking expand opens the first available top panel when file is hidden", () => {
462+
const expandButton = screen.getByTitle("sidebar.expand");
463+
fireEvent.click(expandButton);
464+
expect(screen.queryByText("downloadPanel.heading")).toBeInTheDocument();
465+
});
446466
});

src/components/Menus/Sidebar/SidebarBar.jsx

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,29 @@ const SidebarBar = (props) => {
2828
const bottomMenuOptions = menuOptions.filter(
2929
(menuOption) => menuOption.position === "bottom",
3030
);
31+
const menuOptionNames = menuOptions.map((menuOption) => menuOption.name);
3132
const viewportIsMobile = useMediaQuery({ query: MOBILE_MEDIA_QUERY });
3233
const isMobile = allowMobileView && viewportIsMobile;
3334

35+
const findFirstExistingOption = (candidateOptions = []) => {
36+
return candidateOptions.find(
37+
(optionName) => optionName && menuOptionNames.includes(optionName),
38+
);
39+
};
40+
3441
const expandPopOut = () => {
35-
const option = instructions.length > 0 ? "instructions" : "file";
36-
toggleOption(option);
42+
const hasInstructions =
43+
Array.isArray(instructions) && instructions.length > 0;
44+
const expandOption = findFirstExistingOption([
45+
hasInstructions ? "instructions" : null,
46+
"file",
47+
topMenuOptions[0]?.name, // top part
48+
menuOptions[0]?.name, // bottom part
49+
]);
50+
51+
if (expandOption) {
52+
toggleOption(expandOption);
53+
}
3754
if (window.plausible) {
3855
// TODO: Make dynamic events for each option or rename this event
3956
window.plausible("Expand file pane");

src/components/Menus/Sidebar/SidebarBar.test.js

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ const menuOptions = (instructions = false) => {
2525
panel: () => {},
2626
},
2727
{
28-
name: "home",
28+
name: "projects",
2929
position: "top",
30-
title: "home_button",
30+
title: "projects_button",
3131
panel: () => {},
3232
},
3333
...(instructions
@@ -43,6 +43,23 @@ const menuOptions = (instructions = false) => {
4343
];
4444
};
4545

46+
const menuOptionsWithoutFile = () => {
47+
return [
48+
{
49+
name: "projects",
50+
position: "top",
51+
title: "projects_button",
52+
panel: () => {},
53+
},
54+
{
55+
name: "download",
56+
position: "top",
57+
title: "download_button",
58+
panel: () => {},
59+
},
60+
];
61+
};
62+
4663
describe("SidebarBar", () => {
4764
describe("Without instructions", () => {
4865
beforeEach(() => {
@@ -59,10 +76,10 @@ describe("SidebarBar", () => {
5976
expect(toggleOption).toHaveBeenCalledWith("file");
6077
});
6178

62-
test("Clicking home button opens home panel", () => {
63-
const homeButton = screen.getByTitle("home_button");
64-
fireEvent.click(homeButton);
65-
expect(toggleOption).toHaveBeenCalledWith("home");
79+
test("Clicking projects button opens projects panel", () => {
80+
const projectsButton = screen.getByTitle("projects_button");
81+
fireEvent.click(projectsButton);
82+
expect(toggleOption).toHaveBeenCalledWith("projects");
6683
});
6784
});
6885

@@ -91,4 +108,23 @@ describe("SidebarBar", () => {
91108
expect(toggleOption).toHaveBeenCalledWith("instructions");
92109
});
93110
});
111+
112+
describe("Without file option", () => {
113+
beforeEach(() => {
114+
render(
115+
<Provider store={store}>
116+
<SidebarBar
117+
menuOptions={menuOptionsWithoutFile()}
118+
toggleOption={toggleOption}
119+
/>
120+
</Provider>,
121+
);
122+
});
123+
124+
test("Clicking expand button falls back to the first top panel", () => {
125+
const expandButton = screen.getByTitle("sidebar.expand");
126+
fireEvent.click(expandButton);
127+
expect(toggleOption).toHaveBeenCalledWith("projects");
128+
});
129+
});
94130
});

src/components/WebComponentProject/WebComponentProject.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ describe("When withSidebar is true", () => {
260260
});
261261

262262
test("Renders the sidebar", () => {
263-
expect(screen.queryByTitle("sidebar.collapse")).toBeInTheDocument();
263+
expect(screen.queryByTitle("sidebar.expand")).toBeInTheDocument();
264264
});
265265

266266
test("Renders the correct sidebar options", () => {

0 commit comments

Comments
 (0)