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

New state-specific calculator design #20

Merged
merged 5 commits into from
Sep 26, 2023
Merged

New state-specific calculator design #20

merged 5 commits into from
Sep 26, 2023

Conversation

oyamauchi
Copy link
Member

Description

This implements most of the spec'd design. There are some significant
pieces still missing. They're tracked in Asana, but just as a summary:

  • Multiple selection of projects.

  • Adding authority logos to the bottom of the page.

  • Handling the case of entering a location outside RI.

  • Adding an optional email field for people to join the mailing list.

In addition, there are some more pieces missing before this frontend
is ready to replace the existing IRA calculator wholesale:

  • Spanish localization

  • Analytics

  • Appearance customizability via CSS

Guide to new code

  • New .html page for the RI calculator, which pretty much just has the
    embed. It's included in the Parcel targets, which means it will be
    built and deployed. It's not ready for the public yet, so it uses a
    meta tag to disable search engine indexing.

  • state-calculator.ts is the custom HTML element. This contains all
    the state management.

  • state-incentive-details.ts is the view layer.

    • A couple of sub-parts are factored out: utility-selector.ts and
      icon-tab-bar.ts.
  • calculator-types-v1.ts corresponds to the shape of the API
    responses. This is manually maintained right now, not generated
    from the API in any way; eventually it'd be good to change that.

  • projects.ts has icons and labels for the various projects.

Test Plan

  • Enter a RI zip code. 02807 for Block Island, 02861 for Providence
    (RI Energy only), 02814 for Pascoag (RIE and PUD available).

  • Fill in the rest of the form, click Calculate. Look at results. In
    the 02814 case, switch to the other utility and make sure the
    incentives reload.

  • Click the pill buttons to switch projects.

  • Click the "Learn more" / "Visit site" buttons.

Resize the window to look at the three possible layouts. (1, 2, or 3
columns of incentive cards.)

## Description

This implements most of the spec'd design. There are some significant
pieces still missing. They're tracked in Asana, but just as a summary:

- Multiple selection of projects.

- Adding authority logos to the bottom of the page.

- Handling the case of entering a location outside RI.

- Adding an optional email field for people to join the mailing list.

In addition, there are some more pieces missing before this frontend
is ready to replace the existing IRA calculator wholesale:

- Spanish localization

- Analytics

- Appearance customizability via CSS

### Guide to new code

- New .html page for the RI calculator, which pretty much just has the
  embed. It's included in the Parcel targets, which means it will be
  built and deployed. It's not ready for the public yet, so it uses a
  meta tag to disable search engine indexing.

- `state-calculator.ts` is the custom HTML element. This contains all
  the state management.

- `state-incentive-details.ts` is the view layer.

  - A couple of sub-parts are factored out: `utility-selector.ts` and
    `icon-tab-bar.ts`.

- `calculator-types-v1.ts` corresponds to the shape of the API
  responses. This is manually maintained right now, not generated
  from the API in any way; eventually it'd be good to change that.

- `projects.ts` has icons and labels for the various projects.

## Test Plan

- Enter a RI zip code. 02807 for Block Island, 02861 for Providence
  (RI Energy only), 02814 for Pascoag (RIE and PUD available).

- Fill in the rest of the form, click Calculate. Look at results. In
  the 02814 case, switch to the other utility and make sure the
  incentives reload.

- Click the pill buttons to switch projects.

- Click the "Learn more" / "Visit site" buttons.

Resize the window to look at the three possible layouts. (1, 2, or 3
columns of incentive cards.)
@vercel
Copy link

vercel bot commented Sep 18, 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 25, 2023 9:52pm

@oyamauchi
Copy link
Member Author

I'm sorry, I know this PR is enormous, but (a) there was a lot of churn in getting it to this state, so I'm glad I wasn't sending out smaller PRs as I went through that; and (b) I don't think it's worth breaking this PR up into smaller ones. I tried to sort out the history of the original branch, but it was just too tangled, and it would have taken a lot of time we don't really have.

src/icons.ts Outdated Show resolved Hide resolved
@@ -150,12 +150,14 @@ export class RewiringAmericaCalculator extends LitElement {
? nothing
: formTemplate(
[
'',
Copy link
Member

Choose a reason for hiding this comment

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

I had to double-check what this was - might help to use a constant for NO_PROJECT, or a comment to clarify that this value isn't used for this version?

src/calculator-form.ts Outdated Show resolved Hide resolved
w: number = 20,
h: number = 20,
) =>
svg`<svg width="${w}" height="${h}" viewBox="0 0 20 20" fill="none" xmlns="http://www.w3.org/2000/svg">
Copy link
Member

Choose a reason for hiding this comment

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

For future consideration, would not block this PR.

I think it might help to organize these svgs as files and use parcel to import them? They'd still be inlined in the resulting build, which I think is right for this use-case since they'll all be displayed (though if they were referenced as img tags instead I think they'd be loaded after the main script, which might also be OK?)

A possible downside is that we'd lose the ability to control the width and height directly in the markup, but I think you can solve that with css if needed. Similarly I think the selection color might be possible to set with css and maybe currentColor (which should be picked up from the prevailing text color already)?

The syntax is:

import svg from 'bundle-text:./logo.svg';

See inlining as a string - https://parceljs.org/languages/svg/

Copy link
Member

Choose a reason for hiding this comment

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

(If we do move these to files, my thinking is that they'd be easier to manage and update (and preview) in a text editor, and also that this file would be easier to navigate and understand.)

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 looked into this a bit and didn't get it right on the first try, so I'll defer it to another PR.

query.append('authority_types', 'federal');
query.append('authority_types', 'state');
query.append('authority_types', 'utility');
query.set('utility', this.utility);
Copy link
Member

Choose a reason for hiding this comment

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

When this is blank do we just use our best guess? I wonder if that would be better as an explicit parameter?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is going to change soon, as I get the non-RI experience implemented. As it is, this code won't execute until this.utility is set; the above _utilitiesTask, upon success, sets this.utility and runs this task.

To be clear, these tasks are invoked imperatively, not in response to property changes as in the existing embed implementation. It was much easier to get working this way, given the need to respond to the utility selector changing, which is outside of the main form.

align-items: center;
justify-content: center;

& .caption {
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 use CSS nesting or is there a build step?

I think it's probably/unfortunately too early for CSS nesting without a polyfill? https://caniuse.com/css-nesting

Copy link
Member Author

Choose a reason for hiding this comment

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

No, this is just nesting that arrives in the browser as-is. This is why I was musing about SCSS a while ago.

I'll convert this to BEM style for now, but I would somewhat rather switch this codebase to another CSS solution, probably either SCSS/similar or Tailwind. (Possible there's something else better, too. I'm not a CSS expert; I know just enough to be dangerous.)

Copy link
Member

Choose a reason for hiding this comment

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

I would somewhat rather switch this codebase to another CSS solution, probably either SCSS/similar or Tailwind.

I'd support us switching to Tailwind. It should allow us to more quickly lift over component and layout styles from the main site(s), as well as share design tokens by configuring the build the same way. And I think it only builds what you use, so it shouldn't be huge.

That said, SCSS or another preprocessor would be fine too.

src/utility-selector.ts Outdated Show resolved Hide resolved
@RandomEtc
Copy link
Member

This looks good to me!

Couple of minor bits of feedback above.

The only blocking feedback is to ensure that this builds as a standalone js file before merging.

I don't think the rest are blocking, but I would like to follow up on the CSS nesting question as well as some of the file organization questions:

I verified that the behavior of the old calculator is unaffected by this PR. (Which I think the cypress test also confirms)

One note on interaction: I think we need loading feedback on the map/title and the utility form as well, and maybe on the calculate button. The load spinner was below the fold for me and I couldn't tell if it was doing anything when I changed the form inputs. Made a note on https://app.asana.com/0/1205057814651000/1205391730235898/f

Last thought - I was wondering if the Rhode Island HTML page should have the same developer-facing intro as the index page. We can probably hold off on that until we're ready to deprecate (or sunset) the old one.

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.

Approved once we have state-calculator.js building standalone. I'd like to quickly try that on https://github.com/rewiringamerica/consumer/pull/402 before merging - feel free to take over that PR if I'm not available to do that myself!

@oyamauchi
Copy link
Member Author

I tried this on your consumer branch, and it works, with the caveat that you have to add the attribute state="RI" to the element. That'll be cleared up soon, though.

@oyamauchi
Copy link
Member Author

Also, final note: this will become publicly accessible on embed.rewiringamerica.org, but there's a noindex meta tag so it shouldn't become searchable.

@oyamauchi oyamauchi merged commit 0f32d67 into main Sep 26, 2023
2 checks passed
@oyamauchi oyamauchi deleted the owen/rhode-island branch September 26, 2023 16:40
@RandomEtc
Copy link
Member

Updated https://github.com/rewiringamerica/consumer/pull/402 to confirm this all works. Great milestone!

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