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: readme page #4145

Merged
merged 40 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
520b28a
Temp replacement of feedback log with new readme page
jamdelion Jan 13, 2025
e580475
Merge branch 'main' into jh/experimental-readme-page
jamdelion Jan 13, 2025
a74aa58
Give readme page its own folder and route
jamdelion Jan 13, 2025
a1383d7
Make flow and team names accessible in page
jamdelion Jan 13, 2025
50fbea7
Change to rich text inputs, add existing getFlowInfo call
jamdelion Jan 14, 2025
0f79dc4
Replace existing description input from service settings
jamdelion Jan 15, 2025
188ecaa
Be able to edit flow summary and flow description including new column
jamdelion Jan 16, 2025
135cc46
Get summary working correctly with zustand reset
jamdelion Jan 16, 2025
59f6dc1
Fix type issue
jamdelion Jan 16, 2025
6408eaf
Merge branch 'main' into jh/experimental-readme-page
jamdelion Jan 16, 2025
f389f3c
Merge branch 'main' into jh/experimental-readme-page
jamdelion Jan 20, 2025
ad1fd62
Fix merge
jamdelion Jan 20, 2025
a9521ec
Add serviceLimitations column and get it working
jamdelion Jan 20, 2025
4d56ed4
Undo linting
jamdelion Jan 20, 2025
6fb504a
Undo linting again
jamdelion Jan 20, 2025
84f8b77
Use FlowTag work for labels
jamdelion Jan 20, 2025
7fde819
Add character counter and aria-describedBy plus refresh window on res…
jamdelion Jan 21, 2025
685aad6
Fix input spacing
jamdelion Jan 21, 2025
20ff912
Add clean-html process to demoUser
jamdelion Jan 21, 2025
945e8e7
Wording changes
jamdelion Jan 21, 2025
fff13fa
tidy up
jamdelion Jan 21, 2025
36f9dee
Update to latest planx-core
jamdelion Jan 22, 2025
4cba8ec
Install latest planx-core
jamdelion Jan 22, 2025
b2bec23
Sort sentence casing
jamdelion Jan 23, 2025
92450d2
Merge branch 'main' into jh/experimental-readme-page
jamdelion Jan 23, 2025
16cc688
Fix editornav tests
jamdelion Jan 23, 2025
4678e0e
Make view only for teamViewers
jamdelion Jan 23, 2025
1e2c8c8
Fix external portal test
jamdelion Jan 23, 2025
987e3e0
Fix some types
jamdelion Jan 27, 2025
91706bb
Bring forward the background colour styling of form
jamdelion Jan 27, 2025
1a85cd8
move hex colours to theme level
jamdelion Jan 27, 2025
a827afe
Extract hex to theme for lightOff
jamdelion Jan 27, 2025
7bbd66b
Remove id prop from flowTag
jamdelion Jan 27, 2025
0d6bca8
Use flowName from store
jamdelion Jan 27, 2025
e5e9c0d
Refactor CharacterCounter getTextLimits
jamdelion Jan 27, 2025
78c6dd3
Change column definition to varchar 120
jamdelion Jan 28, 2025
4de0d58
Merge branch 'main' into jh/experimental-readme-page
jamdelion Jan 28, 2025
c90d4b0
Make textfield value optional
jamdelion Jan 28, 2025
fbfd644
Try to fix textInput tests
jamdelion Jan 28, 2025
5c61b9f
Fix text field tests
jamdelion Jan 28, 2025
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
2 changes: 1 addition & 1 deletion editor.planx.uk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"@mui/material": "^5.15.10",
"@mui/utils": "^5.15.11",
"@opensystemslab/map": "1.0.0-alpha.4",
"@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#90601e1",
"@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#cb7246d",
"@tiptap/core": "^2.4.0",
"@tiptap/extension-bold": "^2.0.3",
"@tiptap/extension-bubble-menu": "^2.1.13",
Expand Down
16 changes: 8 additions & 8 deletions editor.planx.uk/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions editor.planx.uk/src/components/EditorNavMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import AdminPanelSettingsIcon from "@mui/icons-material/AdminPanelSettings";
import FactCheckIcon from "@mui/icons-material/FactCheck";
import FormatListBulletedIcon from "@mui/icons-material/FormatListBulleted";
import GroupIcon from "@mui/icons-material/Group";
import Info from "@mui/icons-material/Info";
import LeaderboardIcon from "@mui/icons-material/Leaderboard";
import PaletteIcon from "@mui/icons-material/Palette";
import RateReviewIcon from "@mui/icons-material/RateReview";
Expand Down Expand Up @@ -170,6 +171,12 @@ function EditorNavMenu() {
route: `/${teamSlug}/${flowSlug}`,
accessibleBy: ["platformAdmin", "teamEditor", "demoUser", "teamViewer"],
},
{
title: "About this service",
Icon: Info,
route: `/${teamSlug}/${flowSlug}/about`,
accessibleBy: ["platformAdmin", "teamEditor", "demoUser"],
},
{
title: "Service settings",
Icon: TuneIcon,
Expand Down
227 changes: 227 additions & 0 deletions editor.planx.uk/src/pages/FlowEditor/ReadMePage/ReadMePage.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,227 @@
import Box from "@mui/material/Box";
import Button from "@mui/material/Button";
import Container from "@mui/material/Container";
import Typography from "@mui/material/Typography";
import { TextInputType } from "@planx/components/TextInput/model";
import { useFormik } from "formik";
import { useToast } from "hooks/useToast";
import capitalize from "lodash/capitalize.js";
import React from "react";
import FlowTag, { FlowTagType, StatusVariant } from "ui/editor/FlowTag";
import InputGroup from "ui/editor/InputGroup";
import InputLegend from "ui/editor/InputLegend";
import RichTextInput from "ui/editor/RichTextInput/RichTextInput";
import SettingsDescription from "ui/editor/SettingsDescription";
import SettingsSection from "ui/editor/SettingsSection";
import { CharacterCounter } from "ui/shared/CharacterCounter";
import Input from "ui/shared/Input/Input";
import InputRow from "ui/shared/InputRow";
import { object, string } from "yup";

import { useStore } from "../lib/store";
import { FlowInformation } from "../utils";

interface ReadMePageProps {
flowSlug: string;
flowInformation: FlowInformation;
}

interface ReadMePageForm {
serviceSummary: string;
serviceDescription: string;
serviceLimitations: string;
}

export const ReadMePage: React.FC<ReadMePageProps> = ({
flowSlug,
flowInformation,
}) => {
const { status: flowStatus } = flowInformation;
const [
flowDescription,
updateFlowDescription,
flowSummary,
updateFlowSummary,
flowLimitations,
updateFlowLimitations,
] = useStore((state) => [
state.flowDescription,
state.updateFlowDescription,
state.flowSummary,
state.updateFlowSummary,
state.flowLimitations,
state.updateFlowLimitations,
]);

const toast = useToast();

const formik = useFormik<ReadMePageForm>({
initialValues: {
serviceSummary: flowSummary || "",
serviceDescription: flowDescription || "",
serviceLimitations: flowLimitations || "",
},
onSubmit: async (values, { setSubmitting, setFieldError }) => {
jamdelion marked this conversation as resolved.
Show resolved Hide resolved
try {
const updateFlowDescriptionPromise = updateFlowDescription(
values.serviceDescription
);
const updateFlowSummaryPromise = updateFlowSummary(
values.serviceSummary
);

const updateFlowLimitationsPromise = updateFlowLimitations(
values.serviceLimitations
);
jamdelion marked this conversation as resolved.
Show resolved Hide resolved

const [descriptionResult, summaryResult, limitationsResult] =
await Promise.all([
updateFlowDescriptionPromise,
updateFlowSummaryPromise,
updateFlowLimitationsPromise,
]);

if (descriptionResult && summaryResult && limitationsResult) {
toast.success("Updated successfully");
} else {
if (!descriptionResult) {
setFieldError(
"serviceDescription",
"Unable to update the flow description. Please try again."
);
}
if (!summaryResult) {
setFieldError(
"serviceSummary",
"Unable to update the service summary. Please try again."
);
}
if (!limitationsResult) {
setFieldError(
"serviceLimitations",
"Unable to update the service limitations. Please try again."
);
}
throw new Error("One or more updates failed");
}
} catch (error) {
console.error("Error updating descriptions:", error);
toast.error(
"An error occurred while updating descriptions. Please try again."
);
} finally {
setSubmitting(false);
}
},
validateOnBlur: false,
validateOnChange: false,
validationSchema: object({
serviceSummary: string().max(
120,
"Service description must be 120 characters or less"
),
}),
});

return (
<Container maxWidth="formWrap">
<SettingsSection>
<Typography variant="h2" component="h3" gutterBottom>
{capitalize(flowSlug.replaceAll("-", " "))}
jamdelion marked this conversation as resolved.
Show resolved Hide resolved
</Typography>

<Box display={"flex"}>
<FlowTag
tagType={FlowTagType.Status}
statusVariant={
flowStatus === "online"
? StatusVariant.Online
: StatusVariant.Offline
}
>
{flowStatus}
</FlowTag>
</Box>
</SettingsSection>
<SettingsSection>
<form onSubmit={formik.handleSubmit}>
<InputGroup flowSpacing>
<InputLegend>Service Description</InputLegend>
jamdelion marked this conversation as resolved.
Show resolved Hide resolved
<SettingsDescription>
<>A short blurb on what this service is.</>
</SettingsDescription>
<Input
multiline
{...formik.getFieldProps("serviceSummary")}
jamdelion marked this conversation as resolved.
Show resolved Hide resolved
id="serviceSummary"
placeholder="Description"
errorMessage={formik.errors.serviceSummary}
inputProps={{
"aria-describedby": "A short blurb on what this service is.",
}}
/>
<CharacterCounter
count={formik.values.serviceSummary.length}
textInputType={TextInputType.Short} // 120 characters
error={Boolean(formik.errors.serviceSummary)}
/>
jamdelion marked this conversation as resolved.
Show resolved Hide resolved
</InputGroup>
<InputGroup flowSpacing>
<InputLegend>What does this service do?</InputLegend>
<SettingsDescription>
<>
A longer description of the service. <br />
<br /> How should the service be used? What does it include? Are
there are any dependencies related to this service?
</>
</SettingsDescription>
<InputRow>
<RichTextInput
inputProps={{
"aria-describedby": "A longer description of the service.",
}}
{...formik.getFieldProps("serviceDescription")}
id="serviceDescription"
placeholder="The service..."
errorMessage={formik.errors.serviceDescription}
/>
</InputRow>
</InputGroup>
<InputGroup flowSpacing>
<InputLegend>Limitations of the Service</InputLegend>
<SettingsDescription>
<>What does this service not include?</>
</SettingsDescription>
<InputRow>
<RichTextInput
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a multiline prop for this component which we should use for both of these RichTextInput fields.

It's recently come up as a usability issue when we allow rich text but only provide a small one line input.

Copy link
Contributor Author

@jamdelion jamdelion Jan 27, 2025

Choose a reason for hiding this comment

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

Thanks I was wondering about this - with the multiline prop the default text area of the input doesn't change, and I've checked instances where the prop is added to other RichTextInputs - e.g. the editor for upload and label and those also still have a single line input. Whether the prop is there or not, the input expands when lots of text is added though.

I do think I'd expect this text box to be large by default though - we probably need to add something so that RichTextInput changes style when the multiline prop is present - will add to Trello

(I've added the prop here in any case).

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah great, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although it still doesn't make the text area larger by default 🤔 Will keep digging!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

E.g. the globalSettings line corresponds to this in the UI - i.e. still appears as a single line input

Screenshot 2025-01-27 at 17 26 33

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep very good spot! There's a bug / issue here where the MUI input props (rows, multiline) are not getting correctly transposed onto their tiptap equivalents in the RichTextInput component.

It would be nice to come back and address this but it's outside the scope of this PR and currently behaves exactly as the rest of the Editor does 👍

inputProps={{
"aria-describedby": "What does this service not include",
}}
{...formik.getFieldProps("serviceLimitations")}
id="serviceLimitations"
errorMessage={formik.errors.serviceLimitations}
placeholder="Limitations"
/>
</InputRow>
</InputGroup>

<Box>
<Button type="submit" variant="contained">
Save
</Button>
<Button
onClick={() => window.location.reload()}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a less drastic option like resetting the form we could go for here? I guess a simple formik.resetForm() won't work as there are multiple fields, some of which may have updated.

Maybe for each unupdated (dirty) field, we can revert to the initial value.

Please see comment in main review - maybe reaching to SettingsForm could help us out here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Daf - the problem here was that because the latest data is fetched when the route loads, we get unexpected behaviour if the reset button reverts to the initial value after a user has saved their changes. I.e. it reverts back to before their saved changes instead of accounting for them, even though they have been updated in the background correctly.
Perhaps a solution would be to refetch the data before calling formik.resetForm once the reset button is clicked 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah we're bumping into the awkward data caching / fetching issue we talked about that tanstack query would resolve.

We probably could reset values for formik.initialValues on success but this is a little janky!

This solution works well for now for sure - I think the reset is a bit of an edge case - and lets make a proper plan for how we could integrate tanstack query 👍

type="reset"
variant="contained"
disabled={!formik.dirty}
color="secondary"
sx={{ ml: 1.5 }}
>
Reset changes
</Button>
</Box>
</form>
</SettingsSection>
</Container>
);
};

This file was deleted.

Loading
Loading