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.

Sign up for GitHub

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: Add Preview/Draft Layout Wrapper for Testing Links #4090

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

feat: Add Preview/Draft Layout Wrapper for Testing Links #4090

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
557c9e9
initial wiring
RODO94 Dec 18, 2024
e9c3fe0
add subdomain logic for published link
RODO94 Dec 18, 2024
8985c45
variable renaming and url refinement
RODO94 Dec 19, 2024
e1349ad
switch window.open with react-navi
RODO94 Dec 19, 2024
bb506d9
add warning page to Draft view
RODO94 Dec 19, 2024
b1f10fc
refine logic in Card component
RODO94 Dec 20, 2024
30fbf51
add param description to Card
RODO94 Dec 20, 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
4 changes: 4 additions & 0 deletions editor.planx.uk/src/@planx/components/shared/Preview/Card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import { useStore } from "pages/FlowEditor/lib/store";
import React, { useEffect } from "react";
import { ApplicationPath } from "types";

import NavigateToPublishedButton from "./NavigateToPublishedButton";
import SaveResumeButton from "./SaveResumeButton";

interface Props {
children: React.ReactNode;
isValid?: boolean;
isTestWrapper?: boolean;
handleSubmit?: (data?: any) => void;
}

Expand Down Expand Up @@ -46,6 +48,7 @@ const Card: React.FC<Props> = ({
children,
isValid = true,
handleSubmit,
isTestWrapper,
...props
}) => {
const theme = useTheme();
Expand Down Expand Up @@ -102,6 +105,7 @@ const Card: React.FC<Props> = ({
</Button>
)}
{showSaveResumeButton && <SaveResumeButton />}
{isTestWrapper && <NavigateToPublishedButton />}
</Box>
</InnerContainer>
</Container>
Expand Down
34 changes: 34 additions & 0 deletions editor.planx.uk/src/@planx/components/shared/Preview/NavigateToPublishedButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import Link from "@mui/material/Link";
import Typography from "@mui/material/Typography";
import React from "react";

import { InnerContainer } from "./SaveResumeButton";
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is now shared, it should be its own component.

Generally we'd try to avoid feature to feature imports like this. The main reason being that when I make a change in future on this component it's not inherently obvious that it's going to have a knock on effect in an unrelated component.

A few options here - we could repeat the style, extract it to its own very simple component, or make a component which is the "or" text option which then takes in a callback function. I think the later is the best option here as it keeps an "or" text options consistent and we could control within the card that it only shows once and doesn't stack as is currently possible.

How about a component which took in a condition (to decide if it's shown or hidden) and a callback (to be called when clicked), which is then passed into the Card?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great comment, thanks for the suggestions here. @DafyddLlyr

This may be an indulgence at the wrong time, but I've been reading a bit on decoupling code for maintainability and thus, taken a slightly different approach using the comment as inspo.

I wanted to keep the handleClick logic within each SaveAndResumeButton and NavigateToPublishedButton so they could expand or become more complex in future without bloating out the Card and it means the components can focus on doing one thing well at a time.

I have put in an extra component, OrNavigationButton to handle the logic of the displaying either button based on a type it receives from the Card component, so it again just does one wee thing well. The Card comp can then do a small function to define what type to show.

Lemme know if I've led myself astray and you're suggestion is a cleaner way of doing what I was hoping to do!


const NavigateToPublishedButton: React.FC = () => {
const testEnvironment = window.location.pathname.endsWith("/draft")
? "/draft"
: "/preview";

const editorLink = window.location.pathname.replace(
testEnvironment,
"/published",
) as `/${string}`;
RODO94 marked this conversation as resolved.
Show resolved Hide resolved
const redirectLink = `${window.location.origin}${editorLink}`;
RODO94 marked this conversation as resolved.
Show resolved Hide resolved

const handleClick = () => {
window.open(redirectLink, "_blank");
};

return (
<InnerContainer>
<Typography variant="body1">or</Typography>
<Link onClick={handleClick} component="button">
<Typography variant="body1" textAlign="left">
Go to the live service
</Typography>
</Link>
</InnerContainer>
);
};

export default NavigateToPublishedButton;
2 changes: 1 addition & 1 deletion editor.planx.uk/src/@planx/components/shared/Preview/SaveResumeButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { ApplicationPath } from "types";

import { contentFlowSpacing } from "./Card";

const InnerContainer = styled(Box)(({ theme }) => ({
export const InnerContainer = styled(Box)(({ theme }) => ({
"& p": {
...contentFlowSpacing(theme),
},
Expand Down
24 changes: 24 additions & 0 deletions editor.planx.uk/src/pages/Preview/TestWarningPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import Box from "@mui/material/Box";
import Card from "@planx/components/shared/Preview/Card";
import { CardHeader } from "@planx/components/shared/Preview/CardHeader/CardHeader";
import React, { PropsWithChildren, useState } from "react";

export const TestWarningPage = ({ children }: PropsWithChildren) => {
const [showChildren, setShowChildren] = useState(false);
return (
<>
{showChildren ? (
children
) : (
RODO94 marked this conversation as resolved.
Show resolved Hide resolved
<Box width="100%">
<Card handleSubmit={() => setShowChildren(true)} isTestWrapper={true}>
<CardHeader
title="This is a test environment"
description="This version of the service is unpublished and for testing only. Do not use it to submit real applications"
></CardHeader>
</Card>
</Box>
)}
</>
);
};
5 changes: 4 additions & 1 deletion editor.planx.uk/src/routes/views/preview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import axios, { AxiosError } from "axios";
import { NaviRequest, NotFoundError } from "navi";
import { useStore } from "pages/FlowEditor/lib/store";
import PublicLayout from "pages/layout/PublicLayout";
import { TestWarningPage } from "pages/Preview/TestWarningPage";
import React from "react";
import { View } from "react-navi";
import { getTeamFromDomain } from "routes/utils";
Expand Down Expand Up @@ -40,7 +41,9 @@ export const previewView = async (req: NaviRequest) => {

return (
<PublicLayout>
<View />
<TestWarningPage>
<View />
</TestWarningPage>
</PublicLayout>
);
};
Expand Down
Loading