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

Non-RI experience #23

Merged
merged 3 commits into from
Sep 29, 2023
Merged

Non-RI experience #23

merged 3 commits into from
Sep 29, 2023

Conversation

oyamauchi
Copy link
Member

@oyamauchi oyamauchi commented Sep 27, 2023

Description

This allows you to enter any ZIP the backend recognizes, and removes
the map+utility selector part if the ZIP is not in RI. It got a little
complicated because of the need to accommodate changing the utility
selector without losing the outline map; see the comment on the
tempState property for more detail on that. There are other options
for dealing with this quirk, but this felt like the straightest path.

One thing I wrestled with is that this would be easier if the
/utilities response included the state; that would mean the whole
tempState hack wouldn't be needed. But that felt too much like
allowing the needs of this specific frontend to dictate the design of
the backend.

Test Plan

Calculate the RI zip 02814, which has multiple utility options. Make
sure the UI progresses straight from loading spinner to fully loaded
with utility selector + incentives. RI Energy should be auto-chosen.

Choose Pascoag in the utility selector; make sure the map+selector
remain visible while the new incentives load.

Calculate the zip 02859, which also has RIE and Pascoag. The
map+selector should disappear while loading, then reappear with
Pascoag still selected; incentives should include Pascoag ones.

Calculate the zip 02116, which is in MA. Only IRA incentives should
appear, with no map+selector.

Calculate a RI zip again; make sure the RI experience reappears.

## Description

This allows you to enter any ZIP the backend recognizes, and removes
the map+utility selector part if the ZIP is not in RI. It got a little
complicated because of the need to accommodate changing the utility
selector without losing the outline map; see the comment on the
`tempState` property for more detail on that. There are other options
for dealing with this quirk, but this felt like the straightest path.

One thing I wrestled with is that this would be easier if the
`/utilities` response included the state; that would mean the whole
`tempState` hack wouldn't be needed. But that felt too much like
allowing the needs of this specific frontend to dictate the design of
the backend.

### Followup

I'll implement the behavior of the `state` attribute on the calculator
element "locking" the calculator to that state in a followup PR.

## Test Plan

Calculate the RI zip 02814, which has multiple utility options. Make
sure the UI progresses straight from loading spinner to fully loaded
with utility selector + incentives. RI Energy should be auto-chosen.

Choose Pascoag in the utility selector; make sure the map+selector
remain visible while the new incentives load.

Calculate the zip 02859, which also has RIE and Pascoag. The
map+selector should disappear while loading, then reappear with
Pascoag still selected; incentives should include Pascoag ones.

Calculate the zip 02116, which is in MA. Only IRA incentives should
appear, with no map+selector.

Calculate a RI zip again; make sure the RI experience reappears.
@vercel
Copy link

vercel bot commented Sep 27, 2023

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

Name Status Preview Updated (UTC)
embed-rewiringamerica-org ✅ Ready (Inspect) Visit Preview Sep 29, 2023 7:16pm

Comment on lines 229 to 231
// If the "state" attribute is set, enforce that some other state's
// incentives aren't shown. But if coverage.state is null, we won't show
// the error: we'll only be showing federal incentives in that case.
Copy link
Member Author

Choose a reason for hiding this comment

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

While writing this, I realized it may not be exactly what we want.

As implemented, if you have state="RI" and enter a zip code in any other state, you'll get the generic experience with IRA incentives only, because we have no state-level coverage in any other state. If we were to add coverage in MA, for example, and you entered a MA zip code, you'd get the error.

I think you could make an argument for this behavior: the core thing it's preventing is showing some other state energy office's logo and incentives on your website.

But there are some weird emergent properties:

  • As we add coverage to more states, state-locked embeds will lose functionality. E.g. if MA has no coverage, you can enter a MA zip in a RI-locked embed to see federal incentives. But when we add coverage to MA, entering a MA zip in a RI-locked embed will start causing an error.
  • Suppose we have coverage in MA but not in CT. If you have a RI-locked embed and enter a MA zip, it will say "that ZIP is not in RI", cool. But if you enter a CT zip, you won't get an error, even though that ZIP is not in RI.

If we want the behavior instead to be straightforwardly "you get an error if the ZIP you enter is in a state other than this.state, regardless of coverage", then the coverage field I added to the API is not quite what we want. We'd have to change it to always reflect the location you entered.

Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Let's work through this from the desired UX?

  • On our site, I believe we don't ever want to limit the scope of the calculator to Rhode Island (or any other state). In that case, we may want to say on the page somewhere that we only have detailed incentives for X, Y, Z states so far, but that the tool works no matter what?
  • On RI OER's site, I believe they would want to limit the tool to RI zip codes? I think it's reasonable in that case that if you enter a non-RI zip code then it's a field level error in the form (requiring async validation) and it prevents you from submitting. I would prefer that to an error, but a generic error message would also be fine so long as it's recoverable with an RI zip code.

I think for RI OER that's the last thing you said: "you get an error if the ZIP you enter is in a state other than this.state, regardless of coverage"? Let's do that if state is specified, but otherwise have it return whatever we have?

I think this is probably worth workshopping with Spenser (fairly urgently) next week, so we can make sure we've got clearly designed and supported paths for each use-case?

Copy link
Member

@RandomEtc RandomEtc left a comment

Choose a reason for hiding this comment

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

LGTM in that it's not strictly worse, it opens up the calculator to returning results for residents outside RI which is what we want for our site.

I think you're right though that the behavior needs adjusting further if state is specified.

Copy link
Member Author

@oyamauchi oyamauchi left a comment

Choose a reason for hiding this comment

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

I'm actually going to take out the state attribute and associated logic for this PR, since it's going to change imminently anyway.

@oyamauchi oyamauchi merged commit 55a965d into main Sep 29, 2023
1 check passed
@oyamauchi oyamauchi deleted the owen/nonri branch September 29, 2023 19:20
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.

2 participants