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

SFR-2239: Remove error codes from error page #536

Merged
merged 3 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

- Update license link for editions, works, and collections to copyright
- Update License page to Copyright and add section for "In Copyright" explanation
- Remove error codes from error page
- Update styling for mobile view of error page

## [0.18.3]

Expand Down
14 changes: 10 additions & 4 deletions src/components/Feedback/Feedback.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { FeedbackContext } from "~/src/context/FeedbackContext";

const DEFAULT_DESCRIPTION_TEXT = "Please share your question or feedback.";
const ERROR_DESCRIPTION_TEXT = "We are here to help!";
const ERROR_NOTIFICATION_TEXT = `You are asking for help or information about a page error.`;

const Feedback: React.FC<any> = ({ location }) => {
const [view, setView] = useState<FeedbackBoxViewType>("form");
Expand All @@ -18,14 +19,19 @@ const Feedback: React.FC<any> = ({ location }) => {
onClose,
isError,
notificationText,
statusCode,
setIsError,
setNotificationText,
} = useContext(FeedbackContext);

useEffect(() => {
if (isError) setDescriptionText(ERROR_DESCRIPTION_TEXT);
else setDescriptionText(DEFAULT_DESCRIPTION_TEXT);
}, [isError]);
if (isError) {
setDescriptionText(ERROR_DESCRIPTION_TEXT);
setNotificationText(ERROR_NOTIFICATION_TEXT);
} else {
setDescriptionText(DEFAULT_DESCRIPTION_TEXT);
}
}, [isError, setNotificationText]);

const onCloseAndReset = () => {
if (isError) setIsError(false);
Expand All @@ -38,7 +44,7 @@ const Feedback: React.FC<any> = ({ location }) => {
values: React.ComponentProps<typeof FeedbackBox>["onSubmit"]
) => {
submitFeedback({
feedback: values.comment,
feedback: isError ? `${statusCode} - ${values.comment}` : values.comment,
Copy link
Contributor

@kylevillegas93 kylevillegas93 Oct 10, 2024

Choose a reason for hiding this comment

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

Looks like statusCode could be null though I hope that's never the case so we may just want to have a default. What are your thoughts?

{statusCode ?? 'Unknown'} - ${values.comment}

Copy link
Contributor

Choose a reason for hiding this comment

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

We may also want to prefix the status code with something like Error Code: or HTTP Status Code: just so folks understand what the status code means!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, good idea!

category: isError ? "Bug" : values.category,
url: location,
email: values.email,
Expand Down
5 changes: 5 additions & 0 deletions src/context/FeedbackContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ type FeedbackContextType = {
setIsError: React.Dispatch<React.SetStateAction<boolean | null>>;
notificationText: string | null;
setNotificationText: React.Dispatch<React.SetStateAction<string | null>>;
statusCode: number | null;
setStatusCode: React.Dispatch<React.SetStateAction<number | null>>;
};

export const FeedbackContext = createContext<FeedbackContextType | undefined>(
Expand All @@ -24,6 +26,7 @@ export const FeedbackProvider: React.FC<{
const { FeedbackBox, isOpen, onOpen, onClose } = useFeedbackBox();
const [isError, setIsError] = useState(null);
const [notificationText, setNotificationText] = useState(null);
const [statusCode, setStatusCode] = useState(null);

return (
<FeedbackContext.Provider
Expand All @@ -36,6 +39,8 @@ export const FeedbackProvider: React.FC<{
setIsError,
notificationText,
setNotificationText,
statusCode,
setStatusCode,
}}
>
{children}
Expand Down
24 changes: 11 additions & 13 deletions src/pages/_error.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,59 +14,53 @@ import { FeedbackContext } from "../context/FeedbackContext";

const ERROR_PERSISTS = " if the error persists.";

const getNotificationText = (statusCode) => {
return `You are asking for help or information about a page error (error code ${statusCode})`;
};

const errorMap = {
500: {
overline: "error 500",
heading: "Something went wrong on our end.",
subText: "We encountered an error while trying to load the page. ",
tryText: "Try refreshing the page or ",
},
404: {
overline: "error 404",
heading: "We couldn't find that page.",
subText:
"The page you were looking for doesn't exist or may have moved elsewhere. ",
tryText: "Try a different URL or ",
},
400: {
Copy link
Contributor

Choose a reason for hiding this comment

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

May want to consider having a default error map.

overline: "error 400",
heading: "There was an unexpected error.",
subText: "We couldn't process your request at this time. ",
tryText: "Try again later or ",
},
};

const Error = ({ statusCode }) => {
const { onOpen, isError, setNotificationText, setIsError } =
const { onOpen, isError, setIsError, setStatusCode } =
useContext(FeedbackContext);

useEffect(() => {
if (!isError) {
setIsError(true);
setNotificationText(getNotificationText(statusCode));
setStatusCode(statusCode);
}
}, [isError, setIsError, setNotificationText, statusCode]);
}, [isError, setIsError, setStatusCode, statusCode]);

return (
<Layout>
<Flex
alignItems="center"
flexDir="column"
paddingLeft="l"
paddingRight="l"
textAlign="center"
role="alert"
>
<Image
src="/images/error-img.png"
alt=""
marginBottom="xl"
marginTop="xxxl"
marginTop={{ base: "xl", md: "xxxl" }}
width={100}
/>
<Text size="overline1">{errorMap[statusCode].overline}</Text>
<Heading marginTop="s" marginBottom="s">
{errorMap[statusCode].heading}
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you may just want guard against unexpected error codes like 502, 503, 403 and go to the default error mapping.

Copy link
Collaborator Author

@jackiequach jackiequach Oct 11, 2024

Choose a reason for hiding this comment

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

Spoke with Apoorva and she recommended that we use the 400 error as default.

</Heading>
Expand All @@ -89,7 +83,11 @@ const Error = ({ statusCode }) => {
</Button>
{ERROR_PERSISTS}
</Box>
<Box width="fit-content" marginTop="xxl" marginBottom="xxxl">
<Box
width="fit-content"
marginTop={{ base: "xl", md: "xxl" }}
marginBottom={{ base: "xxl", md: "xxxl" }}
>
<Link to="/" linkType="buttonPrimary">
Back to Digital Research Books
</Link>
Expand Down
Loading