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

feat: auto-answer one at a time, not into the future #3764

Merged
merged 34 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
30139f2
questions are working
jessicamcinchak Oct 4, 2024
3911c82
add autoAnswerableOptions store method, checklists working too
jessicamcinchak Oct 5, 2024
63db51a
filters are working
jessicamcinchak Oct 6, 2024
fddef39
automate through no result flag if no matching flag
jessicamcinchak Oct 6, 2024
0078c28
handle granular automations
jessicamcinchak Oct 6, 2024
32da380
Merge branch 'main' of github.com:theopensystemslab/planx-new into je…
jessicamcinchak Oct 9, 2024
d8ff09c
Merge branch 'main' of github.com:theopensystemslab/planx-new into je…
jessicamcinchak Oct 11, 2024
0495d9e
update upcomingCardIds tests
jessicamcinchak Oct 11, 2024
a1339c1
Merge branch 'main' of github.com:theopensystemslab/planx-new into je…
jessicamcinchak Oct 11, 2024
ccac584
Merge branch 'main' of github.com:theopensystemslab/planx-new into je…
jessicamcinchak Oct 15, 2024
89ad20b
filters work across all flagsets
jessicamcinchak Oct 15, 2024
65cfd79
fix imports
jessicamcinchak Oct 15, 2024
37b2f74
Merge branch 'main' of github.com:theopensystemslab/planx-new into je…
jessicamcinchak Oct 17, 2024
c917afd
stub out filter tests
jessicamcinchak Oct 17, 2024
95c28af
Merge branch 'main' of github.com:theopensystemslab/planx-new into je…
jessicamcinchak Oct 21, 2024
19465b1
more tests
jessicamcinchak Oct 21, 2024
59bf5d9
automate question & checklist blanks
jessicamcinchak Oct 22, 2024
9cc07d2
adjust blanks to account for previously visited options
jessicamcinchak Oct 23, 2024
fadcf03
handle _nots
jessicamcinchak Oct 23, 2024
81bd7f0
Merge branch 'main' of github.com:theopensystemslab/planx-new into je…
jessicamcinchak Oct 24, 2024
9475e74
add forceSelection toggle to Questions & Checklists
jessicamcinchak Oct 24, 2024
a77cca5
separate store methods for auto-answering options v flags, blanks work
jessicamcinchak Oct 25, 2024
348b851
Merge branch 'main' of github.com:theopensystemslab/planx-new into je…
jessicamcinchak Oct 25, 2024
738ff99
tweak parent/child granularity matching
jessicamcinchak Oct 26, 2024
65d8375
lots of tests
jessicamcinchak Oct 28, 2024
7bba548
blanks and Calculate tests
jessicamcinchak Oct 28, 2024
9b522e6
tidy up
jessicamcinchak Oct 28, 2024
efe731d
planning constraints _nots test suite
jessicamcinchak Oct 28, 2024
570cb28
autoAnswerableOptions test todos
jessicamcinchak Oct 29, 2024
91c9dfd
test suite using SetValue and forceSelection prop
jessicamcinchak Oct 29, 2024
a60b5aa
final test suite
jessicamcinchak Oct 29, 2024
d9c94cf
fix: Apply `wasVisited` class to Filter node (#3869)
DafyddLlyr Oct 29, 2024
d699da6
Merge branch 'main' of github.com:theopensystemslab/planx-new into je…
jessicamcinchak Oct 29, 2024
d96476c
first round of PR feedback
jessicamcinchak Oct 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion editor.planx.uk/src/@planx/components/Checklist/Public.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import ImageButton from "@planx/components/shared/Buttons/ImageButton";
import Card from "@planx/components/shared/Preview/Card";
import CardHeader from "@planx/components/shared/Preview/CardHeader";
import { getIn, useFormik } from "formik";
import React, { useState } from "react";
import { useStore } from "pages/FlowEditor/lib/store";
import React, { useEffect, useState } from "react";
import { ExpandableList, ExpandableListItem } from "ui/public/ExpandableList";
import FormWrapper from "ui/public/FormWrapper";
import FullWidthWrapper from "ui/public/FullWidthWrapper";
Expand All @@ -38,6 +39,37 @@ function toggleInArray<T>(value: T, arr: Array<T>): Array<T> {
}

const ChecklistComponent: React.FC<Props> = (props) => {
const autoAnswerableOptions = useStore(
(state) => state.autoAnswerableOptions,
);

let idsThatCanBeAutoAnswered: string[] | undefined;
if (props.id) idsThatCanBeAutoAnswered = autoAnswerableOptions(props.id);

if (idsThatCanBeAutoAnswered && idsThatCanBeAutoAnswered.length > 0) {
jessicamcinchak marked this conversation as resolved.
Show resolved Hide resolved
return (
<AutoAnsweredChecklist {...props} answerIds={idsThatCanBeAutoAnswered} />
);
} else {
return <VisibleChecklist {...props} />;
}
jessicamcinchak marked this conversation as resolved.
Show resolved Hide resolved
};

// An auto-answered Checklist won't be seen by the user, but still leaves a breadcrumb
const AutoAnsweredChecklist: React.FC<Props & { answerIds: string[] }> = (
props,
) => {
useEffect(() => {
props.handleSubmit?.({
answers: props.answerIds,
auto: true,
});
}, []);

return null;
};

const VisibleChecklist: React.FC<Props> = (props) => {
jessicamcinchak marked this conversation as resolved.
Show resolved Hide resolved
const {
description = "",
groupedOptions,
Expand Down
26 changes: 26 additions & 0 deletions editor.planx.uk/src/@planx/components/Filter/Public.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import type { PublicProps } from "@planx/components/ui";
import { useStore } from "pages/FlowEditor/lib/store";
import { useEffect } from "react";

import type { Filter } from "./model";

export type Props = PublicProps<Filter>;

// A Filter is always auto-answered and never seen by a user, but should still leave a breadcrumb
export default function Component(props: Props) {
const autoAnswerableOptions = useStore(
(state) => state.autoAnswerableOptions,
);

let idsThatCanBeAutoAnswered: string[] | undefined;
if (props.id) idsThatCanBeAutoAnswered = autoAnswerableOptions(props.id);

useEffect(() => {
props.handleSubmit?.({
answers: idsThatCanBeAutoAnswered,
auto: true,
});
}, []);

return null;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ import FormLabel from "@mui/material/FormLabel";
import Grid from "@mui/material/Grid";
import RadioGroup from "@mui/material/RadioGroup";
import { visuallyHidden } from "@mui/utils";
import { Edges } from "@opensystemslab/planx-core/types";
import Card from "@planx/components/shared/Preview/Card";
import CardHeader from "@planx/components/shared/Preview/CardHeader";
import BasicRadio from "@planx/components/shared/Radio/BasicRadio";
import DescriptionRadio from "@planx/components/shared/Radio/DescriptionRadio";
import ImageRadio from "@planx/components/shared/Radio/ImageRadio";
import { useFormik } from "formik";
import React from "react";
import { useStore } from "pages/FlowEditor/lib/store";
import React, { useEffect } from "react";
import FormWrapper from "ui/public/FormWrapper";
import FullWidthWrapper from "ui/public/FullWidthWrapper";
import ErrorWrapper from "ui/shared/ErrorWrapper";
Expand All @@ -24,6 +26,45 @@ export enum QuestionLayout {
}

const QuestionComponent: React.FC<Question> = (props) => {
const [flow, autoAnswerableOptions] = useStore((state) => [
state.flow,
state.autoAnswerableOptions,
]);

// Questions without edges function like 'sticky notes' in the graph for editors only
let edges: Edges | undefined;
if (props.id) edges = flow[props.id]?.edges;
if (!edges || edges.length === 0) {
return <AutoAnsweredQuestion {...props} answerIds={undefined} />;
}
jessicamcinchak marked this conversation as resolved.
Show resolved Hide resolved

let idsThatCanBeAutoAnswered: string[] | undefined;
if (props.id) idsThatCanBeAutoAnswered = autoAnswerableOptions(props.id);

if (idsThatCanBeAutoAnswered && idsThatCanBeAutoAnswered.length > 0) {
return (
<AutoAnsweredQuestion {...props} answerIds={idsThatCanBeAutoAnswered} />
);
} else {
return <VisibleQuestion {...props} />;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Higher level comment, just discussed on dev call.

This repeated logic may be best at the level above this (Node.tsx). It's possible we could pass an prop into each component via getComponentProps() - maybe isAutoAnswered or isVisible or something meaningful.

This would allow us to have simpler components and more centralised logic over visibility. The visibility logic would be component specific in some circumstances - for example, filters and checklists have different requirements. I think this would make it feel like checking for visibility/auto answering is a global display rule of the graph, with many implementations (which it is I belive), as opposed to a series of exceptions sort of hidden away within each component.

Worth a look as a follow up PR to this once, or at least worth another look before we jump into applying this pattern to other component types I think.

Copy link
Member Author

@jessicamcinchak jessicamcinchak Oct 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very relevant questions & agree there's pieces of code here that already indicate that Node.tsx is probably the better place to handle this (eg the initial guard clauses in the store methods you point out), but very in favor of exploring in a follow-up PR to this one because I think it's a larger question than only components I've touched here!

For example:

  • How would it work for Questions, Checklists, Filters updated here?
  • How would it work for other existing component types which are already auto-answered / hidden from users like SetValues, Calculates, Pay when fee = 0 etc?

Let's keep this one "unresolved" & experiment with this before we try out input automations soon 👍

};

// An auto-answered Question won't be seen by the user, but still leaves a breadcrumb
const AutoAnsweredQuestion: React.FC<
Question & { answerIds: string[] | undefined }
> = (props) => {
useEffect(() => {
props.handleSubmit?.({
answers: props.answerIds,
auto: true,
});
}, []);

return null;
};

const VisibleQuestion: React.FC<Question> = (props) => {
const previousResponseId = props?.previouslySubmittedData?.answers?.[0];
const previousResponseKey = props.responses.find(
(response) => response.id === previousResponseId,
Expand Down
Loading
Loading