Add a redesigned 404 / 503 error page (#2840)

* Add a redesigned 404 / 503 error page This commit updates and unifies some of our error pages to make them prettier and more informative. This commit also removes accidental suppression of 404 errors. In the past, visiting a non-existent URL _without_ a locale (eg. https://voice.mozilla.org/this-is-a-bad-url) would silently fail and redirect to a localized homepage. The old 404 page could only be seen when a locale was already included in the URL (eg. https://voice.mozilla.org/en/this-is-a-bad-url). Now, _any_ bad URL will redirect to /<locale>/404, locale or not. --- Note 1: I'm torn about this decision, but for simplicity I've updated the URL to `/:locale/:errorCode` whenever a 404 or 503 are encountered. I personally prefer for the old URL to hang around, because it makes wayfinding and refreshing easier. The following reasons made me opt for an explicit error code in the URL: • We currently assign page-level classNames based on the URL. To get custom page CSS (eg. layout, hiding `#help-links`) we need a page-level classname. To do this without the help of the URL would require custom Redux work for this page alone, and it seemed like too big of a change for this initial commit. • Our routing happens in several layers due to localization, and makes heavy use of the `<Redirect />` component. Following this pattern also requires we keep the error state in the URL. I kinda hate it, but I don't hate it hate it. Let me know if you hate it hate it and I can make a ticket in Jira, but for now it seems pretty low priority. --- Note 2: since the 503 page is shown whenever we hit the top-level `componentDidCatch`, it might appear for client-side errors. Since `componentDidCatch` is only triggered, I'd wager client-side errors will _usually_ be the cause. --- Note 3: The new design removes the report + reload functionality from our previous barebones error page. I'm fine releasing as-is for now, but very happy to discuss if you've got ideas about whether to re-introduce reports, or flows where the new error page might cause problems. --- Note 4: While working on this commit, Prettier and I made some small cleanup fixes (removed unused imports, used CSS variable names, etc). So as not to complicate the review, I'm going to add those in a follow-on commit called "Misc cleanup" – feel free to skip that one during review. --- Test plan: • Navigate to https://localhost:9000/en/this-is-a-bad-url 404 page should appear. • Navigate to https://localhost:9000/this-is-a-bad-url 404 page should appear. • Trigger the top-level `componentDidCatch` handler 503 page should appear. In all above tests, layout should be responsive and animations performant. * Misc cleanup * Add custom GA tracking to error pages
common-voice · Jul 25, 2020 · 7e2fc0c · 7e2fc0c
1 parent c01551d
commit 7e2fc0c
Show file tree

Hide file tree

Showing 23 changed files with 363 additions and 75 deletions.
diff --git a/server/src/server.ts b/server/src/server.ts
@@ -366,10 +366,7 @@ export default class Server {
     this.print('acquiring lock');
     const lock = await redlock.lock(
       'common-voice-maintenance-lock',
-      1000 *
-        60 *
-        60 *
-        6 /* keep lock for 6 hours */
+      1000 * 60 * 60 * 6 /* keep lock for 6 hours */
     );
     // we need to check again after the lock was acquired, as another instance
     // might've already migrated in the meantime

diff --git a/web/locales/en/messages.ftl b/web/locales/en/messages.ftl
@@ -501,9 +501,12 @@ sst-explanation = Speech-to-text (STT) technologies convert voice data into text
 de-identified = De-identified
 de-identified-explanation = The process by which a contributor’s profile information is obscured from their donated voice clips when packaged for download as a part of the dataset.
 
-## NotFound
-notfound-title = Not found
-notfound-content = I’m afraid I don’t know what you’re looking for.
+## Error pages
+error-title-404 = We couldn’t find that page for you
+error-content-404 = Maybe our <homepageLink>homepage</homepageLink> will help? To ask a question, please join the <matrixLink>Matrix community chat</matrixLink>, monitor site issues via <githubLink>GitHub</githubLink> or visit <discourseLink>our Discourse forums</discourseLink>.
+error-title-503 = We’re experiencing unexpected downtime
+error-content-503 = The site will be back up as soon as possible. For the latest information, please join the <matrixLink>Matrix community chat</matrixLink> or visit <githubLink>GitHub</githubLink> or <discourseLink>our Discourse forums</discourseLink> to submit and monitor site experience issues.
+error-code = Error { $code }
 
 ## Data
 data-download-button = Download Common Voice Data

diff --git a/web/src/components/app.tsx b/web/src/components/app.tsx
@@ -35,7 +35,6 @@ import StateTree from '../stores/tree';
 import { Uploads } from '../stores/uploads';
 import { User } from '../stores/user';
 import Layout from './layout/layout';
-import NotificationBanner from './notification-banner/notification-banner';
 import NotificationPill from './notification-pill/notification-pill';
 import { Spinner } from './ui/ui';
 import {
@@ -348,8 +347,11 @@ class App extends React.Component {
   }
 
   async componentDidCatch(error: Error, errorInfo: any) {
-    this.setState({ error });
-
+    this.setState({ error }, () =>
+      history.push(`${this.userLocales[0]}/503`, {
+        prevPath: history.location.pathname,
+      })
+    );
     if (!isProduction() && !isStaging()) return;
 
     Sentry.withScope(scope => {
@@ -362,20 +364,7 @@ class App extends React.Component {
   }
 
   render() {
-    const { error, Sentry } = this.state;
-    if (error) {
-      return (
-        <div>
-          An error occurred. Sorry!
-          <br />
-          <button onClick={() => Sentry.showReportDialog()} disabled={!Sentry}>
-            Report feedback
-          </button>
-          <br />
-          <button onClick={() => location.reload()}>Reload</button>
-        </div>
-      );
-    }
+    const userLocale = this.userLocales[0];
 
     return (
       <Suspense fallback={<Spinner />}>
@@ -388,9 +377,7 @@ class App extends React.Component {
                   exact
                   path={url || '/'}
                   render={() => (
-                    <Redirect
-                      to={'/' + this.userLocales[0] + url + location.search}
-                    />
+                    <Redirect to={`/${userLocale}${url}${location.search}`} />
                   )}
                 />
               ))}
@@ -406,9 +393,21 @@ class App extends React.Component {
                   match: {
                     params: { locale },
                   },
-                }) => (
-                  <LocalizedPage userLocales={[locale, ...this.userLocales]} />
-                )}
+                }) =>
+                  LOCALES.includes(locale) ? (
+                    <LocalizedPage
+                      userLocales={[locale, ...this.userLocales]}
+                    />
+                  ) : (
+                    <Redirect
+                      push
+                      to={{
+                        pathname: `/${userLocale}/404`,
+                        state: { prevPath: location.pathname },
+                      }}
+                    />
+                  )
+                }
               />
             </Switch>
           </Router>

diff --git a/web/src/components/invite-modal/invite-modal.css b/web/src/components/invite-modal/invite-modal.css
@@ -46,7 +46,7 @@
         }
 
         .icon {
-            color: #4a4a4a;
+            color: var(--near-black);
             margin-left: 12px;
             font-size: 0.6em;
         }

diff --git a/web/src/components/layout/content.tsx b/web/src/components/layout/content.tsx
@@ -3,7 +3,6 @@ import { Switch, Route, Redirect } from 'react-router';
 import URLS from '../../urls';
 import { isContributable, useLocale } from '../locale-helpers';
 import DocumentPage from '../pages/document-page';
-import NotFoundPage from '../pages/not-found';
 import { Spinner } from '../ui/ui';
 import { LoginFailure, LoginSuccess } from '../pages/login';
 const HomePage = React.lazy(() => import('../pages/home/home'));
@@ -14,8 +13,9 @@ const ProfileLayoutPage = React.lazy(() => import('../pages/profile/layout'));
 const FAQPage = React.lazy(() => import('../pages/faq/faq'));
 const AboutPage = React.lazy(() => import('../pages/about/about'));
 const LandingPage = React.lazy(() => import('../pages/landing/landing'));
+const ErrorPage = React.lazy(() => import('../pages/error-page/error-page'));
 
-export default function Content() {
+export default function Content({ location }: { location: any }) {
   const [locale, toLocaleRoute] = useLocale();
   return (
     <div id="content">
@@ -115,7 +115,25 @@ export default function Content() {
             path={toLocaleRoute('/landing/sodedif')}
             component={LandingPage}
           />
-          <Route component={NotFoundPage} />
+          <Route
+            path={toLocaleRoute('/404')}
+            render={() => (
+              <ErrorPage errorCode="404" prevPath={location.state?.prevPath} />
+            )}
+          />
+          <Route
+            path={toLocaleRoute('/503')}
+            render={() => (
+              <ErrorPage errorCode="503" prevPath={location.state?.prevPath} />
+            )}
+          />
+          <Redirect
+            push
+            to={{
+              pathname: toLocaleRoute('/404'),
+              state: { prevPath: location.pathname },
+            }}
+          />
         </Switch>
       </React.Suspense>
     </div>

diff --git a/web/src/components/layout/layout.tsx b/web/src/components/layout/layout.tsx
@@ -87,7 +87,7 @@ const SegmentBanner = ({
       <>
         <Localized
           id="target-segment-first-banner"
-          vars={{locale: NATIVE_NAMES[locale]}}
+          vars={{ locale: NATIVE_NAMES[locale] }}
         />
       </>
     ),
@@ -110,7 +110,10 @@ const SegmentBanner = ({
           ),
         },
         {
-          href: locale === 'es' ? URLS.TARGET_SEGMENT_INFO_ES : URLS.TARGET_SEGMENT_INFO,
+          href:
+            locale === 'es'
+              ? URLS.TARGET_SEGMENT_INFO_ES
+              : URLS.TARGET_SEGMENT_INFO,
           blank: true,
           persistAfterClick: true,
           className: 'cta external',
@@ -167,8 +170,8 @@ class Layout extends React.PureComponent<LayoutProps, LayoutState> {
     });
 
     try {
-      FullStory.setUserVars({ isLoggedIn: !!user.account })
-    } catch(e) {
+      FullStory.setUserVars({ isLoggedIn: !!user.account });
+    } catch (e) {
       // do nothing if FullStory not initialized (see app.tsx)
     }
   }
@@ -227,7 +230,7 @@ class Layout extends React.PureComponent<LayoutProps, LayoutState> {
     trackGlobal('change-language', locale);
     setLocale(locale);
     this.setState({
-       featureStorageKey: await this.getFeatureKey(locale)
+      featureStorageKey: await this.getFeatureKey(locale),
     });
     history.push(replacePathLocale(history.location.pathname, locale));
   };
@@ -271,7 +274,9 @@ class Layout extends React.PureComponent<LayoutProps, LayoutState> {
     } = this.state;
     const isBuildingProfile = location.pathname.includes(URLS.PROFILE_INFO);
 
-    const pathParts = location.pathname.split('/');
+    const pathParts = location.pathname
+      .replace(/(404|503)/g, 'error-page')
+      .split('/');
     const className = cx(pathParts[2] ? pathParts.slice(2).join(' ') : 'home', {
       'staging-banner-is-visible': showStagingBanner,
     });
@@ -292,9 +297,13 @@ class Layout extends React.PureComponent<LayoutProps, LayoutState> {
             teamToken={challengeTeamToken}
           />
         )}
-        {featureStorageKey && localStorage.getItem(featureStorageKey) !== 'true' && (
-          <SegmentBanner locale={locale} featureStorageKey={featureStorageKey} />
-        )}
+        {featureStorageKey &&
+          localStorage.getItem(featureStorageKey) !== 'true' && (
+            <SegmentBanner
+              locale={locale}
+              featureStorageKey={featureStorageKey}
+            />
+          )}
         {showStagingBanner && (
           <div className="staging-banner">
             You're on the staging server. Voice data is not collected here.{' '}
@@ -363,7 +372,7 @@ class Layout extends React.PureComponent<LayoutProps, LayoutState> {
             this.scroller = div as HTMLElement;
           }}>
           <div id="scrollee">
-            <Content />
+            <Content location={location} />
             <Footer />
           </div>
         </div>

diff --git a/web/src/components/pages/about/get-involved.tsx b/web/src/components/pages/about/get-involved.tsx
@@ -20,8 +20,9 @@ const GetInvolved: React.ComponentType = () => {
     <>
       <img
         className="wave-footer"
-        src={require('./images/wave-footer@3x.png')}
-        alt="Wave"
+        src={require('../images/wave-footer@3x.png')}
+        alt=""
+        role="presentation"
       />
 
       {/*<div className="become-partner">
@@ -49,7 +50,7 @@ const GetInvolved: React.ComponentType = () => {
           elems={{
             discourseLink: <DiscourseLink />,
             githubLink: <GitHubLink />,
-            matrixLink: <MatrixLink />
+            matrixLink: <MatrixLink />,
           }}>
           <p />
         </Localized>

diff --git a/web/src/components/pages/dashboard/challenge/challenge.css b/web/src/components/pages/dashboard/challenge/challenge.css
@@ -72,15 +72,15 @@
     & .info {
         display: flex;
         flex-direction: row;
-        color: #4a4a4a;
+        color: var(--near-black);
         padding-right: 40%;
 
         & svg {
             margin-right: 15px;
         }
 
         & path {
-            fill: #4a4a4a;
+            fill: var(--near-black);
         }
 
         & a {
@@ -621,7 +621,11 @@
 }
 
 .speak-btn::after {
-    background: linear-gradient(90deg, var(--gradient-pink), var(--gradient-purple));
+    background: linear-gradient(
+        90deg,
+        var(--gradient-pink),
+        var(--gradient-purple)
+    );
 }
 .listen-btn::after {
     background: linear-gradient(270deg, #88d1f1, #b1b5e5);

diff --git a/web/src/components/pages/dashboard/challenge/weekly-challenge.css b/web/src/components/pages/dashboard/challenge/weekly-challenge.css
@@ -41,7 +41,7 @@
     margin: 0 7.5px;
     &.active {
         color: var(--white);
-        background-color: #4a4a4a;
+        background-color: var(--near-black);
     }
 }
 .weekly-content {