Skip to content

Commit 5f4e9d4

Browse files
committed
Add alert to show validation errors
1 parent 6b965af commit 5f4e9d4

5 files changed

Lines changed: 167 additions & 3 deletions

File tree

extensions/ql-vscode/src/stories/method-modeling/MethodModeling.stories.tsx

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,35 @@ MultipleModelingsModeledMultiple.args = {
6969
showMultipleModels: true,
7070
modelingStatus: "saved",
7171
};
72+
73+
export const MultipleModelingsValidationFailedNeutral = Template.bind({});
74+
MultipleModelingsValidationFailedNeutral.args = {
75+
method,
76+
modeledMethods: [
77+
createModeledMethod(method),
78+
createModeledMethod({
79+
...method,
80+
type: "neutral",
81+
}),
82+
],
83+
showMultipleModels: true,
84+
modelingStatus: "unsaved",
85+
};
86+
87+
export const MultipleModelingsValidationFailedDuplicate = Template.bind({});
88+
MultipleModelingsValidationFailedDuplicate.args = {
89+
method,
90+
modeledMethods: [
91+
createModeledMethod(method),
92+
createModeledMethod({
93+
...method,
94+
type: "source",
95+
input: "",
96+
output: "ReturnValue",
97+
kind: "remote",
98+
}),
99+
createModeledMethod(method),
100+
],
101+
showMultipleModels: true,
102+
modelingStatus: "unsaved",
103+
};

extensions/ql-vscode/src/view/common/Alert.tsx

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,28 @@ type Props = {
8585

8686
// Inverse the color scheme
8787
inverse?: boolean;
88+
89+
/**
90+
* Role is used as the ARIA role. "alert" should only be set if the alert requires
91+
* the user's immediate attention. "status" should be set if the alert is not
92+
* important enough to require the user's immediate attention.
93+
*
94+
* Can be left out if the alert is not important enough to require the user's
95+
* immediate attention.
96+
*/
97+
role?: "alert" | "status";
8898
};
8999

90-
export const Alert = ({ type, title, message, actions, inverse }: Props) => {
100+
export const Alert = ({
101+
type,
102+
title,
103+
message,
104+
actions,
105+
inverse,
106+
role,
107+
}: Props) => {
91108
return (
92-
<Container type={type} inverse={inverse}>
109+
<Container type={type} inverse={inverse} role={role}>
93110
<Title>
94111
{getTypeText(type)}: {title}
95112
</Title>
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { ModeledMethodValidationError } from "../../model-editor/shared/validation";
2+
import TextButton from "../common/TextButton";
3+
import { Alert } from "../common";
4+
import * as React from "react";
5+
import { useCallback } from "react";
6+
7+
type Props = {
8+
error: ModeledMethodValidationError;
9+
setSelectedIndex: (index: number) => void;
10+
};
11+
12+
export const ModeledMethodAlert = ({ error, setSelectedIndex }: Props) => {
13+
const handleClick = useCallback(() => {
14+
setSelectedIndex(error.index);
15+
}, [error.index, setSelectedIndex]);
16+
17+
return (
18+
<Alert
19+
role="alert"
20+
type="error"
21+
title={error.title}
22+
message={
23+
<>
24+
{error.message}{" "}
25+
<TextButton onClick={handleClick}>{error.actionText}</TextButton>
26+
</>
27+
}
28+
/>
29+
);
30+
};

extensions/ql-vscode/src/view/method-modeling/MultipleModeledMethodsPanel.tsx

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import * as React from "react";
2-
import { useCallback, useState } from "react";
2+
import { useCallback, useMemo, useState } from "react";
33
import { Method } from "../../model-editor/method";
44
import { ModeledMethod } from "../../model-editor/modeled-method";
55
import { styled } from "styled-components";
66
import { MethodModelingInputs } from "./MethodModelingInputs";
77
import { VSCodeButton } from "@vscode/webview-ui-toolkit/react";
88
import { Codicon } from "../common";
9+
import { validateModeledMethods } from "../../model-editor/shared/validation";
10+
import { ModeledMethodAlert } from "./ModeledMethodAlert";
911

1012
export type MultipleModeledMethodsPanelProps = {
1113
method: Method;
@@ -19,9 +21,14 @@ const Container = styled.div`
1921
gap: 0.25rem;
2022
2123
padding-bottom: 0.5rem;
24+
border-top: 0.05rem solid var(--vscode-panelSection-border);
2225
border-bottom: 0.05rem solid var(--vscode-panelSection-border);
2326
`;
2427

28+
const AlertContainer = styled.div`
29+
margin-top: 0.5rem;
30+
`;
31+
2532
const Footer = styled.div`
2633
display: flex;
2734
flex-direction: row;
@@ -47,8 +54,24 @@ export const MultipleModeledMethodsPanel = ({
4754
setSelectedIndex((previousIndex) => previousIndex + 1);
4855
}, []);
4956

57+
const validationErrors = useMemo(
58+
() => validateModeledMethods(modeledMethods),
59+
[modeledMethods],
60+
);
61+
5062
return (
5163
<Container>
64+
{validationErrors.length > 0 && (
65+
<AlertContainer>
66+
{validationErrors.map((error, index) => (
67+
<ModeledMethodAlert
68+
key={index}
69+
error={error}
70+
setSelectedIndex={setSelectedIndex}
71+
/>
72+
))}
73+
</AlertContainer>
74+
)}
5275
{modeledMethods.length > 0 ? (
5376
<MethodModelingInputs
5477
method={method}

extensions/ql-vscode/src/view/method-modeling/__tests__/MultipleModeledMethodsPanel.spec.tsx

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,16 @@ describe(MultipleModeledMethodsPanel.name, () => {
194194
}),
195195
).toHaveValue("source");
196196
});
197+
198+
it("does not show errors", () => {
199+
render({
200+
method,
201+
modeledMethods,
202+
onChange,
203+
});
204+
205+
expect(screen.queryByRole("alert")).not.toBeInTheDocument();
206+
});
197207
});
198208

199209
describe("with three modeled methods", () => {
@@ -309,4 +319,56 @@ describe(MultipleModeledMethodsPanel.name, () => {
309319
).toHaveValue("remote");
310320
});
311321
});
322+
323+
describe("with duplicate modeled methods", () => {
324+
const modeledMethods = [
325+
createModeledMethod({
326+
...method,
327+
}),
328+
createModeledMethod({
329+
...method,
330+
}),
331+
];
332+
333+
it("shows errors", () => {
334+
render({
335+
method,
336+
modeledMethods,
337+
onChange,
338+
});
339+
340+
expect(screen.getByRole("alert")).toBeInTheDocument();
341+
});
342+
343+
it("shows the correct error message", async () => {
344+
render({
345+
method,
346+
modeledMethods,
347+
onChange,
348+
});
349+
350+
expect(
351+
screen.getByText("Error: Duplicated classification"),
352+
).toBeInTheDocument();
353+
expect(
354+
screen.getByText(
355+
"This method has two identical or conflicting classifications.",
356+
),
357+
).toBeInTheDocument();
358+
359+
expect(screen.getByText("1/2")).toBeInTheDocument();
360+
361+
const button = screen.getByText(
362+
"Modify or remove the duplicated classification.",
363+
);
364+
365+
await userEvent.click(button);
366+
367+
expect(screen.getByText("2/2")).toBeInTheDocument();
368+
369+
expect(
370+
screen.getByText("Modify or remove the duplicated classification."),
371+
).toBeInTheDocument();
372+
});
373+
});
312374
});

0 commit comments

Comments
 (0)