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

Add byline support to Text Introductions #7367

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

Conversation

chosak
Copy link
Member

@chosak chosak commented Nov 23, 2022

(This PR incorporates and rebases #7348.)

This commit adds support for bylines to the Text Introduction block in the same way that Item Introduction blocks support them. To avoid duplication, the common byline functionality has been pulled out into a new byline template (although not into a new Wagtail block).

All three of the affected templates (byline, item introduction, text introduction) can be previewed using this repository's template debug functionality at these URLs:

Screenshots of these URLs are below.

Like Item Introduction bylines, Text Introduction bylines can have zero or more authors and an optional date.

This commit also includes some minor cleanup around the way that item introduction blocks have been manually rendered on various page types. As far as I can tell this special handling is no longer needed, if it ever was. Affected page types and example pages to verify this (compare against production):

This PR also adds a few new Python unit tests to validate the intended template rendering.

Screenshots

Byline Item Introduction Text Introduction
image image image

Notes and todos

  • Adding byline support to Text Intros is a change that should be reflected in the CFPB Design System. The existing Text Introduction page is here; this is already out of date based on other changes that have happened over the years (for example adding an optional eyebrow in Add optional 'eyebrow' to Text Introduction module #4004).
  • Similarly, the existing Item Introduction page in the DS (here) is also out of date; for example, we no longer link authors, and we want to make icons non-optional, as mentioned by @jenn-franklin on Fix: Item introduction categories don't link #7360 (comment).
  • Other design system questions for @jenn-franklin @sonnakim: does it make sense to add the byline as a new component, with guidance around, say, how author names should appear and how dates should be formatted? Looking at the II and TI screenshots above, these two are now quite similar; do we feel like these have drifted too close together or need to be better disambiguated?
  • I like to use the template debug feature because it provides an easy way to iterate and preview our components in different configurations. @anselmbradford @wpears we've discussed in the past that it would be nice if this could live in the DS itself, so just wanted to flag that as part of our ongoing DS developer experience work.

csebianlander and others added 2 commits November 28, 2022 08:44
This commit adds support for bylines to the Text Introduction block
in the same way that Item Introduction blocks support them. To avoid
duplication, the common byline functionality has been pulled out into
a new byline template (although not into a new Wagtail block).

All three of the affected templates (byline, item introduction, text
introduction) can be previewed using this repository's template debug
functionality at these URLs:

- http://localhost:8000/admin/template_debug/v1/byline/
- http://localhost:8000/admin/template_debug/v1/item_introduction/
- http://localhost:8000/admin/template_debug/v1/text_introduction/

Like Item Introduction bylines, Text Introduction bylines can have
zero or more authors and an optional date.

This commit also includes some minor cleanup around the way that
item introduction blocks have been manually rendered on various page
types. As far as I can tell this special handling is no longer needed,
if it ever was. Affected page types and example pages to verify this:

- BlogPage: http://localhost:8000/about-us/blog/why-were-modernizing-how-we-collect-credit-card-data/
- DocumentDetailPage: http://localhost:8000/compliance/supervisory-guidance/bulletin-phone-pay-fees/
- EnforcementActionPage: http://localhost:8000/enforcement/actions/capital-one-bank/
- LearnPage: http://localhost:8000/enforcement/information-industry-whistleblowers/privacy-act-statement/
- NewsroomPage: http://localhost:8000/about-us/newsroom/cfpb-issues-guidance-to-address-shoddy-investigation-practices-by-consumer-reporting-companies/
- HMDAHistoricDataPage: http://localhost:8000/data-research/hmda/historic-data/

This commit also adds a few new Python unit tests to validate the
intended template rendering.
@chosak
Copy link
Member Author

chosak commented Nov 28, 2022

Adding the "hold" label to this PR per discussion in standup today. @sonnakim @csebianlander are going to further discuss stakeholder needs to see whether adding byline support to TIs is the best path forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants