Skip to content

Commit

Permalink
Merge pull request #407 from MeasureAuthoringTool/MAT-7792-fix
Browse files Browse the repository at this point in the history
MAT-7792: address feedback
  • Loading branch information
mcmcphillips authored Dec 12, 2024
2 parents 9c93636 + 034f721 commit 5ce034a
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ interface ArgumentsProps {
canEdit: boolean;
addArgumentToFunctionsArguments: Function;
deleteArgumentFromFunctionArguments: Function;
dirty: boolean;
}

const availableDataTypes = [
Expand All @@ -43,8 +44,8 @@ export default function ArgumentSection(props: ArgumentsProps) {
deleteArgumentFromFunctionArguments,
functionArguments,
canEdit,
dirty,
} = props;
const [functionDataType, setFunctionDataType] = useState("");
const [confirmationDialog, setConfirmationDialog] = useState<boolean>(false);

const formik = useFormik({
Expand All @@ -60,8 +61,8 @@ export default function ArgumentSection(props: ArgumentsProps) {
const { dataType } = formik.values;

const handleSubmit = () => {
let value = formik.values.dataType;
if (functionDataType === "Other") {
let value = dataType;
if (value === "Other") {
value = formik.values.other;
}
const fnToAdd = {
Expand All @@ -72,7 +73,12 @@ export default function ArgumentSection(props: ArgumentsProps) {
formik.resetForm();
};
const { resetForm } = formik;

// reset inner if outer is clean (listen for reset event)
useEffect(() => {
if (!dirty) {
resetForm();
}
}, [dirty]);
return (
<>
<div tw="flex flex-wrap">
Expand Down Expand Up @@ -118,14 +124,15 @@ export default function ArgumentSection(props: ArgumentsProps) {
error={Boolean(formik.errors.dataType)}
helperText={formik.errors.dataType}
onChange={(evt) => {
setFunctionDataType(evt.target.value);
formik.setFieldValue("dataType", evt.target.value);
formik.setFieldValue("other", "");
if (formik.values.other) {
formik.setFieldValue("other", "");
}
}}
/>
</div>
</div>
{functionDataType && functionDataType === "Other" && (
{dataType === "Other" && (
<div tw="flex flex-wrap">
<div tw="pt-6 w-1/2">
<TextField
Expand Down Expand Up @@ -181,7 +188,6 @@ export default function ArgumentSection(props: ArgumentsProps) {
onClose={() => setConfirmationDialog(false)}
onSubmit={() => {
resetForm();
setFunctionDataType("");
setConfirmationDialog(false);
}}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -449,9 +449,6 @@ describe("CQL Function Builder Tests", () => {
const functionNameInput = (await screen.findByTestId(
"function-name-text-input"
)) as HTMLInputElement;
const argumentsSection = screen.getByTestId(
"terminology-section-Arguments-sub-heading"
);
expect(functionNameInput).toBeInTheDocument();
expect(functionNameInput.value).toBe("");
fireEvent.change(functionNameInput, {
Expand Down Expand Up @@ -522,29 +519,48 @@ describe("CQL Function Builder Tests", () => {
});
expect(argumentNameInput.value).toBe("Test");

// dataType
const dataTypeDropdown = await screen.findByTestId(
"arg-type-selector-input"
);
fireEvent.change(dataTypeDropdown, {
target: { value: "Other" },
});

// we now need to populate other textfield
const otherNameInput = (await screen.findByTestId(
"other-type-input"
)) as HTMLInputElement;
expect(otherNameInput).toBeInTheDocument();
expect(otherNameInput.value).toBe("");
fireEvent.change(otherNameInput, {
target: { value: "Other" },

// switch to blank other
fireEvent.change(dataTypeDropdown, {
target: { value: "Integer" },
});
await waitFor(() => {
expect(screen.queryByTestId("other-type-input")).not.toBeInTheDocument();
});
expect(otherNameInput.value).toBe("Other");

const addButton = screen.getByTestId("function-argument-add-btn");
expect(addButton).toBeInTheDocument();
expect(addButton).toBeEnabled();
// select other again
fireEvent.change(dataTypeDropdown, {
target: { value: "Other" },
});
// other input appears
await waitFor(() => {
expect(screen.queryByTestId("other-type-input")).toBeInTheDocument();
});

fireEvent.click(addButton);
// finda nd fill out other
const other = (await screen.findByTestId(
"other-type-input"
)) as HTMLInputElement;
fireEvent.change(other, {
target: { value: "test" },
});
await waitFor(() => {
const addButton = screen.getByTestId("function-argument-add-btn");
expect(addButton).toBeEnabled();
fireEvent.click(addButton);
});

const functionArgumentTable = screen.getByTestId("function-argument-tbl");
expect(functionArgumentTable).toBeInTheDocument();
Expand All @@ -565,7 +581,7 @@ describe("CQL Function Builder Tests", () => {
functionsArguments: [
expect.objectContaining({
argumentName: "Test",
dataType: "Other",
dataType: "test",
}),
],
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export default function FunctionBuilder({
useState<boolean>(false);
const textAreaRef = useRef(null);
const [confirmationDialog, setConfirmationDialog] = useState<boolean>(false);
const [expressionEditorValue, setExpressionEditorValue] = useState("");
const [cursorPosition, setCursorPosition] = useState(null);
const [autoInsert, setAutoInsert] = useState(false);
const formik = useFormik({
Expand All @@ -59,6 +58,7 @@ export default function FunctionBuilder({
comment: funct?.comment || "",
fluentFunction: funct?.fluentFunction || true,
functionsArguments: funct?.functionsArguments || [],
expressionEditorValue: "",
type: "",
name: "",
},
Expand All @@ -68,20 +68,21 @@ export default function FunctionBuilder({
const newValues = getNewExpressionsAndLines(
values,
cursorPosition,
expressionEditorValue,
formik.values.expressionEditorValue,
autoInsert
);
updateExpressionAndLines(newValues[0], newValues[1]);
},
});
const { resetForm } = formik;
// going to pass dirty down to know when we need to reset sub form
const { resetForm, dirty } = formik;

// update formik, and expressionEditor, cursor, lines
const updateExpressionAndLines = (
newEditorExpressionValue,
newCursorPosition
) => {
setExpressionEditorValue(newEditorExpressionValue);
formik.setFieldValue("expressionEditorValue", newEditorExpressionValue);
formik.setFieldValue("type", "");
formik.setFieldValue("name", "");

Expand Down Expand Up @@ -186,6 +187,7 @@ export default function FunctionBuilder({
deleteArgumentFromFunctionArguments={
deleteArgumentFromFunctionArguments
}
dirty={dirty}
functionArguments={formik.values.functionsArguments}
/>
</ExpandingSection>
Expand All @@ -197,8 +199,10 @@ export default function FunctionBuilder({
expressionEditorOpen={expressionEditorOpen}
cqlBuilderLookupsTypes={cqlBuilderLookupsTypes}
textAreaRef={textAreaRef}
expressionEditorValue={expressionEditorValue}
setExpressionEditorValue={setExpressionEditorValue}
expressionEditorValue={formik.values.expressionEditorValue}
setExpressionEditorValue={(v) => {
formik.setFieldValue("expressionEditorValue", v);
}}
setCursorPosition={setCursorPosition}
setAutoInsert={setAutoInsert}
/>
Expand All @@ -225,10 +229,9 @@ export default function FunctionBuilder({
comment: formik.values.comment,
functionsArguments: formik.values.functionsArguments,
fluentFunction: formik.values.fluentFunction,
expressionValue: expressionEditorValue,
expressionValue: formik.values.expressionEditorValue,
};
resetForm();
setExpressionEditorValue("");
handleApplyFunction(functionToApply);
}}
>
Expand Down

0 comments on commit 5ce034a

Please sign in to comment.