PDF cert warning added to find-form search

[jtmst]

Description

Adds an info banner to the find-a-form search page

Original issue(s)

department-of-veterans-affairs/va.gov-cms#11021

Testing done

Tested locally both pointed at staging and prod to test flipper logic functionality

QA Steps:
1: Point local api to prod to verify that the alert does not render, to staging to see that it does
2. Navigate to /find-forms/
3. Verify that the alert renders above the va-introtext

Screenshots

Acceptance Criteria

• Environment flag/Flipper prevents from showing in Prod
• Text is exactly what appears above in description
• Only shows if Flipper is set to true in the environment
• Alert displays under H1 when user lands on Form Search page
• Alert persists when user executes a search
• Alert is not dismissible
• Screen reader reads the alert only once on the Search page while the user remains there (without navigating away), even as the user executes one or more searches
• Requires design review
• Requires accessibility review

Definition of done

• Events are logged appropriately
• Documentation has been updated, if applicable
• A link has been provided to the originating GitHub issue (or connected to it via ZenHub)
• No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs

[va-vfs-bot]

Icon found

Icons can be decorative, but sometimes they are used to convey meaning. If there are any semantics associated with an icon, those semantics should also be conveyed to a screen reader.

What you can do

Review the markup and see if the icon provides information that isn't represented textually, or wait for a VSP review.

[va-vfs-bot]

icon found

[va-vfs-bot]

icon found

[va-vfs-bot]

icon found

[va-vfs-bot]

ESLint is disabled

vets-website uses ESLint to help enforce code quality. In most situations we would like ESLint to remain enabled.

What you can do

See if the code can be refactored to avoid disabling ESLint, or wait for a VSP review.

[va-vfs-bot]

ESLint disabled here

[laflannery]

This looks good to me, tested with NVDA as well

[apisandipas]

This looks good to me.

[jilladams]

@wesrowe flag: the copy in our ticket doesn't match the copy Laura provided in department-of-veterans-affairs/va.gov-team#48458 (comment). Can you confirm whether our ticket copy is ok to merge, or needs to be updated to use the SWC copy provided?

[wesrowe]

You said "okay to merge" – reminder that this should not be merged since that's our strategy for holding it back for Collab Cycle review.

So to your question about content...
What happened on the Search page is that SWContent proposed an update to the copy to include a "dec 15" date when all forms will have been updated, and I missed the github comment. We don't have any confirmation that is going to be the date, so I don't think we should commit to it to Vets. So for the Search page, really I think it's safe to release as it is in the PR and the issue Josh worked from. (And go to Staging Review with that version.)

Same for the Form Detail page. SWContent proposed adding the "Dec 15" – which would be a nice "next steps" detail to offer if it were safe to offer it. But we can't commit at this time to Dec 15 resolution of last few forms. So for that PR I say stick with the copy we have.

[jilladams]

Good catch, thx.

[ryguyk]

CONSIDER
React provides a first-class way of handling this sort of thing, called portals. It allows us a way to render a child component outside of its parent's DOM hierarchy while keeping the component within the parent's React tree. In this case, we do still need to create a DOM node that we can set as the target of the portal, so we will still have some imperative, side-effect code to create that node (i.e. document.createElement). The code that actually renders the content, though, remains declarative, first-class React code.

const PdfAlert = () => {
  const pdfCertWarningId = 'pdf-cert-warning';
  const pdfCertWarningElement = document.getElementById(pdfCertWarningId);
  const [
    pdfCertWarningElementCreated,
    setPdfCertWarningElementCreated,
  ] = useState(false);

  useEffect(() => {
    const domNode = document.createElement('div');
    domNode.setAttribute('id', pdfCertWarningId);

    const introTextNode = document.getElementsByClassName('va-introtext')[0];
    const introTextParentNode = introTextNode?.parentNode;
    if (introTextNode && introTextParentNode) {
      introTextParentNode.insertBefore(domNode, introTextNode);
      setPdfCertWarningElementCreated(true);
    }
  }, []);

  if (!pdfCertWarningElementCreated || !pdfCertWarningElement) {
    return <></>;
  }

  return ReactDOM.createPortal(
    <va-alert>
      <h2 slot="headline" className="vads-u-font-size--h3">
        We’re updating our forms
      </h2>
      <p>
        After January 7, 2023, you won’t be able to use VA forms that have a
        “last updated” date before March 2022. If you downloaded any of these
        older VA forms, you may need to download new copies in January.
      </p>
    </va-alert>,
    pdfCertWarningElement,
  );
};

export const FindVaForms = ({ showPdfWarningBanner }) => {
  return (
    <>
      {showPdfWarningBanner && <PdfAlert />}
      <SearchForm />
      <SearchResults />
      ...
      ...
    </>
  );

This change is not critical, especially for code that is temporary. It actually makes the code longer, as well, but it's more idiomatic. We'd need to implement in this fashion, say, if we needed events to bubble from the child component up to the parent. As it currently is implemented, the "child" is completely removed from its React "parent".

[ryguyk]

It looks like the fallback (... || false) might be in the wrong place here. I think it should be outside the brackets.

That said, it might be even better to define a default value for this property on the component definition and then just leave this map function to set a possibly undefined value.

export const FindVaForms = ({ showPdfWarningBanner = false }) => {

[jtmst]

good call, on it