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

Exclude IRA rebates; add ability to show generic info about HEAR/HER #155

Merged
merged 4 commits into from
Apr 30, 2024

Conversation

oyamauchi
Copy link
Member

Description

This excludes all federal non-tax-credit incentives that come from the
API, and adds in these visually-distinct cards with less-specific info
about HEAR rebates if the API response indicates the user is below
150% AMI.

One slight difference from the design spec is that the description
text is grey-500, rather than 400, which fails the a11y test for not
having enough contrast with the yellow background. I can't even see
the difference at a glance, so I think it's fine. (As a drive-by, I
fixed a couple places where we had gray instead of grey.)

Once we settle on copy, I'll start the translation workflow.

We don't have any state-specific logic yet, but there's a clear place
to add it once we figure out what exactly we want for each state. Some
states will want us to show nothing at all (either because they don't
want to commit to anything or because they already have actual HEAR
programs represented in the API), some states may want semi-specific
info in advance of rolling out actual programs, some may tell us
they're going to exclude certain items, etc. All that can be built on
this foundation.

As background, I don't want to put this in the API because the kind of
info we want for HEAR just doesn't fit within the current API shape:
it's too vague and uncertain. I don't want to extend the API shape to
encompass stuff like this because I think it's valuable, on principle,
to hold the line that the API only returns concrete, actionable
incentives that are certain, or close to certain, to be available.

https://app.asana.com/0/1206661332626418/1206925887004341

Test Plan

Cypress tests pass.

Copy link

vercel bot commented Apr 24, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
embed-rewiringamerica-org ✅ Ready (Inspect) Visit Preview Apr 30, 2024 3:18pm

Copy link
Member

Choose a reason for hiding this comment

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

Does this test also ensure that something from the API is displayed too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, all the checks below are for API-sourced incentives.

## Description

This excludes all federal non-tax-credit incentives that come from the
API, and adds in these visually-distinct cards with less-specific info
about HEAR rebates if the API response indicates the user is below
150% AMI.

One slight difference from the design spec is that the description
text is grey-500, rather than 400, which fails the a11y test for not
having enough contrast with the yellow background. I can't even see
the difference at a glance, so I think it's fine. (As a drive-by, I
fixed a couple places where we had `gray` instead of `grey`.)

Once we settle on copy, I'll start the translation workflow.

We don't have any state-specific logic yet, but there's a clear place
to add it once we figure out what exactly we want for each state. Some
states will want us to show nothing at all (either because they don't
want to commit to anything or because they already have actual HEAR
programs represented in the API), some states may want semi-specific
info in advance of rolling out actual programs, some may tell us
they're going to exclude certain items, etc. All that can be built on
this foundation.

As background, I don't want to put this in the API because the kind of
info we want for HEAR just doesn't fit within the current API shape:
it's too vague and uncertain. I don't want to extend the API shape to
encompass stuff like this because I think it's valuable, on principle,
to hold the line that the API only returns concrete, actionable
incentives that are certain, or close to certain, to be available.

https://app.asana.com/0/1206661332626418/1206925887004341

## Test Plan

Cypress tests pass.
Copy link
Contributor

@matelau matelau left a comment

Choose a reason for hiding this comment

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

The code LGTM! But, man the card is a lot harder to appreciate it on page. It is the Yellow on Yellow, that is rough to look at. Is it too late to consider an alternative? I know this is in alignment with the design, but it really make a diff seeing it on the page.
image

image

@@ -164,4 +164,15 @@ export const templates = {
sfc7214f623fe475d: `Selecciona la empresa a la que paga su factura de electricidad.`,
sfe16afc784bb9d76: `Tejado solar`,
sfe81d5d73a35a2cf: `Calcular`,
sb1ef6ac20f1ddfff: `Discount off an electric panel`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these just placeholders as we wait for the translated strings? How do we usually manage translations?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was waiting until we got agreement on copy to request translations. This is just what happens if you run yarn strings:build without translations: the original string gets put in as a fallback. I'll wait until we get translations to land this.

@oyamauchi oyamauchi merged commit a134a94 into main Apr 30, 2024
3 checks passed
@oyamauchi oyamauchi deleted the owen.state-specific branch April 30, 2024 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants