reCAPTCHA: Error pages #2585

machikoyasuda · 2024-12-11T17:24:02Z

What this PR does

Realign all the error pages
Refactors error pages so they all inherit from 1 base file, now called error-base.html (previously was error.html). Now both Admin and Benefits have an error-base. The base naming is used like this now:
, to let the developer know that this is a template that inherits from base.html, and other templates then inherit it.
error-base.html now makes it so that there is one place to make these changes, making it easier to review also. When making a new error page that is based on this template/design, the developer should not have to write any padding/margin/centering/alignment classes or code. They should only have to write copy (headline-text), create a new or use an existing includes for an icon, add any <p> of text, and use an existing button, or create a new one (the button might have existing CSS classes, like login). But there shouldn't be a need for any new rows or justify-content-centers or col-lg-6. That is the responsibility of the error-base template.

How `error-base` works:

{% extends "core/base.html" %}  /* inherits from base, so any blocks in base can be used - even if not specified here */
{% load i18n %}

{% block page-title %}
  {% block title %}
  {% endblock title %}
{% endblock page-title %}

/* inherits from base, so you could put a block for nav-buttons here to have a nice area to put the log out button */

{% block inner-content %}   /* inherits from base, `inner-content` has a `<div class="row justify-content-center`"> outside of it, so we can omit it. you have 2 options here: override inner-content entirely, OR, onlly override the children: icon, headline-text, paragraphs. all of our error pages now do this, but in the future if there is some weird new one-off error design where these elements are in a different order, we can override inner-content directly*/
  <div class="col-lg-6">
    <div class="mt-5 pt-4 text-center">
      {% block icon %} /* put the icon includes in here directly, no wrapper div needed */
      {% endblock icon %} /* Default: sadbus icon. If sadbus, then don't need to include this block at all */
    </div>
    <h1 class="h2 py-4">
      {% block headline-text %} /* put the text in here directly, no wrapper div needed */
      {% endblock headline-text %}
    </h1>
    {% block paragraphs %} /* put paragraph elements and other includes (like agency-links) in here directly, no wrapper div needed */
    {% endblock paragraphs %}
  </div>
{% endblock inner-content %}

{% block call-to-action-button %} /* the logic here is, "show the button to home page by default if a template doesn't specify one. or override the button by putting in your own block. */
  {% include "core/includes/button--origin.html" %}
{% endblock call-to-action-button %}

Error pages

500
404
400
200 user error
OAuth system error
Enrollment system error
Enrollment retry
Re-enrollment CalFresh
Eligibility unverified

Side note outside of the scope of this PR/Milestone

The transit agency name is supposed to be bold on the agency-links, but it's not. Not part of this PR.
I noticed an inconsistency in button text casing. There's Start Here and Return home and Try again. Not part of this PR.
Some files are set to 4 space tabs, while most are 2: reenrollment-error--calfresh and start--medicare at least. Not sure why. Not part of this PR.
Testing these pages is kinda cumbersome. Perhaps we could have some sort of helper file / method that makes it easier to access all of them.

github-actions · 2024-12-11T17:25:09Z

Coverage report

This PR does not seem to contain any modification to coverable code.

machikoyasuda · 2024-12-11T17:38:48Z

benefits/static/css/variables.css

I didn't change or touch this file at all.... the linter did this!

machikoyasuda · 2024-12-11T19:03:07Z

benefits/templates/error-base.html

@@ -0,0 +1,26 @@
+{% extends "core/base.html" %}
+{% load i18n %}


Maybe this {% load i18n %} is unnecessary???

Seems like it, there are no {% translate %} tags on this base page.

machikoyasuda · 2024-12-11T19:04:00Z

benefits/templates/error-base.html

+{% block page-title %}
+  {% block title %}
+  {% endblock title %}
+{% endblock page-title %}


Is this necessary? Test.

I wouldn't think so. I would think the inheriting error page e.g. 404.html or whatever could just use the
{% block page-title %} directly, right?

So this was not necessary, but I had to then go back to each 40X error page and change page-title to `title.

This anti-pattern is also exists on landing/agency-index (which btw is confusing naming. landing-base/agency-landing might be less confusing.

thekaveman · 2024-12-12T00:14:17Z

benefits/templates/200-user-error.html

 {% load i18n %}

 {% block title %}
  {% translate "Start over" %}
 {% endblock title %}

-{% block headline %}
+{% block icon %}
+  {% include "core/includes/icon.html" with name="sadbus" %}


I love this refactor to have all error pages inherit from error-base.html 😍

I think this icon include could be moved there as the "default" icon. Which means it wouldn't have to be repeated on all these regular error pages.

That doesn't prevent us from overriding it on error pages that do need a custom icon.

Default sad bus.

thekaveman · 2024-12-12T00:14:57Z

benefits/templates/error-base.html

+  <div class="col-lg-6">
+    <div class="mt-5 pt-4 text-center">
+      {% block icon %}
+      {% endblock icon %}


See comment above about a "default" icon here.

machikoyasuda · 2024-12-12T19:02:12Z

I noticed that the transit agency name bolding on agency links is a regression on main. See more here: #2590 So I made a bug ticket and I can work on it after reCAPTCHA is done, in this sprint or immediately in slush.

… 500 error pages

…calfresh

…ys error pg

… error-base

…from base

machikoyasuda requested a review from a team as a code owner December 11, 2024 17:24

github-actions bot added tests Related to automated testing (unit, UI, integration, etc.) front-end HTML/CSS/JavaScript and Django templates and removed tests Related to automated testing (unit, UI, integration, etc.) labels Dec 11, 2024

machikoyasuda force-pushed the feat/2545-recaptcha-errors branch from 37c0630 to 817bae4 Compare December 11, 2024 17:31

machikoyasuda commented Dec 11, 2024

View reviewed changes

machikoyasuda self-assigned this Dec 11, 2024

machikoyasuda mentioned this pull request Dec 11, 2024

reCAPTCHA: Text padding realignment #2540

Open

3 tasks

machikoyasuda commented Dec 11, 2024

View reviewed changes

thekaveman requested changes Dec 12, 2024

View reviewed changes

machikoyasuda force-pushed the feat/2545-recaptcha-errors branch from e24f966 to b47401d Compare December 12, 2024 01:23

machikoyasuda requested a review from thekaveman December 12, 2024 01:25

machikoyasuda added 15 commits December 12, 2024 11:16

feat(error-template): realign error template for 200 error, 400, 404,…

fa88233

… 500 error pages

feat(oauth-error): realign oauth system error page

e55fd29

feat(reenrollment-calfresh): realign template for reenrollment error …

01112e9

…calfresh

fix: remove unnecessary outer div, provided by base

9a9c646

feat(enrollment-error): realign template, update css for enrollment s…

8a301c6

…ys error pg

feat(enrollment-retry): realign template for reenrollment retry error

2acaf46

feat(elig-unverified): realign eligibility unverified error page

732f794

refactor(templates): move log out button into nav-buttons

f2f7358

refactor(error): all error pages now extend error

a85e8d9

refactor: rename error to error-base, for consistency

4e6eeab

test(models): update test_tempate_path spec

5b91916

refactor(oauth-error): remove unnecessary button call, inherited from…

9ed5091

… error-base

refactor(error-base): loading i18n not necessary

46fd496

refactor(error-base): remove redundant title call and use page-title …

2a69f17

…from base

refactor(error-base): set sad bus icon as the default icon

812446c

machikoyasuda force-pushed the feat/2545-recaptcha-errors branch from d38b680 to 812446c Compare December 12, 2024 19:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

reCAPTCHA: Error pages #2585

reCAPTCHA: Error pages #2585

machikoyasuda commented Dec 11, 2024 •

edited

Loading

github-actions bot commented Dec 11, 2024 •

edited

Loading

machikoyasuda Dec 11, 2024

machikoyasuda Dec 11, 2024 •

edited

Loading

thekaveman Dec 12, 2024

machikoyasuda Dec 12, 2024

machikoyasuda Dec 11, 2024

thekaveman Dec 12, 2024

machikoyasuda Dec 12, 2024

thekaveman Dec 12, 2024

machikoyasuda Dec 12, 2024

machikoyasuda Dec 12, 2024

machikoyasuda Dec 12, 2024

thekaveman Dec 12, 2024 •

edited

Loading

machikoyasuda commented Dec 12, 2024

		@@ -0,0 +1,26 @@
		{% extends "core/base.html" %}
		{% load i18n %}

reCAPTCHA: Error pages #2585

Are you sure you want to change the base?

reCAPTCHA: Error pages #2585

Conversation

machikoyasuda commented Dec 11, 2024 • edited Loading

What this PR does

How error-base works:

Error pages

Side note outside of the scope of this PR/Milestone

github-actions bot commented Dec 11, 2024 • edited Loading

Coverage report

Choose a reason for hiding this comment

machikoyasuda Dec 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thekaveman Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

machikoyasuda commented Dec 12, 2024

machikoyasuda commented Dec 11, 2024 •

edited

Loading

How `error-base` works:

github-actions bot commented Dec 11, 2024 •

edited

Loading

machikoyasuda Dec 11, 2024 •

edited

Loading

thekaveman Dec 12, 2024 •

edited

Loading