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.

Sign up for GitHub

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

Jump to bottom

feat: Create weather widget #1079

Merged
merged 73 commits into from
Sep 1, 2023
Merged

feat: Create weather widget #1079

merged 73 commits into from
Sep 1, 2023

Conversation

jcbcapps
Copy link
Contributor

@jcbcapps jcbcapps commented Aug 7, 2023 •
edited
Loading

SC-1444

Proposed changes

  • Add Weather Widget to My Space

TO DO:

  • Unit tests
  • E2E tests
  • Add documentation for adding a new widget
  • Update Storybook component to use mock data
  • Add LD flag

Reviewer notes

All you need to do in order to test:

  • Click the 'Add widget' component in My Space and select 'Add weather widget'
  • You should see a new widget appear, prompting you to enter a zip code
  • After saving the zip code, you should see the weather appear
  • You can click the settings icon in the top right to edit/remove
  • You can also click the zip code itself to edit
  • You should also be able to drag-and-drop weather widgets

Currently, since the rate limit for the National Weather Service API is not public knowledge, I have limited the number of allowed weather widgets to 3. That is semi-arbitrary and I welcome feedback on that decision.

Check out the e2e tests: https://github.com/USSF-ORBIT/ussf-portal/pull/308

Setup

IMPORTANT:
In order for this to work, you need to have the sc-2156-backend-weather-widget branch checked out in the CMS repo (at least until it's merged into main)

Start the system

yarn services:up
yarn dev
cd ../ussf-portal-cms
yarn dev

Login to the portal http://localhost:3000

Start storybook

yarn storybook

Login to storybook http://localhost:6006, though the command above should open it for you

Code review steps

As the original developer, I have

  • Met the acceptance criteria
  • Created new stories in Storybook if applicable
  • Created/modified automated unit tests in Jest
  • Created/modified automated E2E tests
  • Followed guidelines for zero-downtime deploys, if applicable
  • Use ANDI to check for basic a11y issues

As a reviewer, I have

Check out our How to review a pull request document.

Screenshots

one
two
three
four
five

jcbcapps and others added 22 commits July 28, 2023 14:57
@jcbcapps
Add baseline component
091a0db
@abbyoung
Add resolvers and mutations for managing weather widget
c5e6aab
@jcbcapps
Add component boilerplate
10e7aa6
@abbyoung
fix copy pasta
ad55be0
@abbyoung
actually use the editWeatherWidget from mySpaceContext
1df88d5
@jcbcapps
Merge
62f5c37
@jcbcapps
Remove unnecessary lines
9a2aa61
@jcbcapps
Remove unused imports and add type
de073b1
@jcbcapps
Update types
db054b5
@jcbcapps
Add weather widget option to AddWidget
85a8ddc
@jcbcapps
Change to DraggableWidget
d564d01
@jcbcapps
Remove prop for AddWidget in favor of using myspaceContext
28dede1
@abbyoung
update edit to change zipcode and coords
2dae958
@abbyoung
remove unused
b281fe6
@abbyoung
refresh weather widget if coords updated
0a7dacd
@abbyoung
removing a comment
762df4a
@jcbcapps
Add TemporaryWidget
c15b839
@jcbcapps
Add WeatherWidget as an option in TemporaryWidget
faf90ba
@jcbcapps
Merge latest api updates
ff0b1f7
@jcbcapps
Pass user-provided zip to mutation and create weather widget in db
e126bb5
@jcbcapps
Clean up and display appropriate weather info
88559b9
@jcbcapps
Move temporaryWidget state
802413e
@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #1444: Create Weather widget.

@shkeating
Copy link
Contributor

shkeating commented Aug 28, 2023 •
edited
Loading

my only notes now are I think the text in the hourly forecast should be aligned center, they appear left aligned rn
image

gidjin
gidjin reviewed Aug 29, 2023
View reviewed changes
src/components/WeatherWidget/WeatherWidget.stories.tsx Outdated
Comment on lines 45 to 47
export const WeatherWidgetWithData = () => (
<WeatherWidget widget={mockWeatherWidget} />
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: We probably should mock out the api call to weather.gov in storybook like we do with the RSS widget. Otherwise we'll have a happo diff every time the weather changes.

Question: should we add mocks to show some of the other weather possibilities?

Copy link
Contributor

Choose a reason for hiding this comment

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

I added code to do this as discussed.

@gidjin gidjin self-requested a review August 29, 2023 21:43
gidjin
gidjin suggested changes Aug 29, 2023
View reviewed changes
Copy link
Contributor

@gidjin gidjin left a comment

Choose a reason for hiding this comment

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

Requesting change to mock out the weather widget with data in storybook, if we can't let me know and I'll approve.

shkeating
shkeating approved these changes Aug 31, 2023
View reviewed changes
Copy link
Contributor

@shkeating shkeating left a comment

Choose a reason for hiding this comment

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

with the hourly forecast centered its lookin good to me!

@shkeating
Copy link
Contributor

Requesting change to mock out the weather widget with data in storybook, if we can't let me know and I'll approve.

+1 to this tho. pretty sure it makes it hard to run storybook by itself if you don't use mock data

gidjin
gidjin approved these changes Sep 1, 2023
View reviewed changes
src/components/WeatherWidget/WeatherWidget.stories.tsx Outdated
Comment on lines 45 to 47
export const WeatherWidgetWithData = () => (
<WeatherWidget widget={mockWeatherWidget} />
)
Copy link
Contributor

Choose a reason for hiding this comment

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

I added code to do this as discussed.

@gidjin gidjin requested a review from shkeating September 1, 2023 04:20
@jcbcapps jcbcapps merged commit 3e6b91d into main Sep 1, 2023
@jcbcapps jcbcapps deleted the sc-1444/create-weather-widget branch September 1, 2023 22:09
@github-actions github-actions bot mentioned this pull request Sep 1, 2023
Merged
gidjin pushed a commit that referenced this pull request Sep 11, 2023
@github-actions
chore(main): release 4.23.0 (#1071)
27c789c 
## [4.23.0](4.22.1...4.23.0) (2023-09-11)


### Features

* Add resolvers and mutations for managing weather widget ([#1075](#1075)) ([a652766](a652766))
* Add support for CTA to link a document ([#1082](#1082)) ([fa0a6bb](fa0a6bb))
* Change h3 from book weight to bold weight ([#1076](#1076)) ([c92ae65](c92ae65))
* Create weather widget ([#1079](#1079)) ([3e6b91d](3e6b91d))
* Load user personnel data into portal ([#1088](#1088)) ([fe0a1d8](fe0a1d8))
* new loader animation ([#1085](#1085)) ([b099904](b099904))


### Bug Fixes

* **deps:** update dependencies ([#1059](#1059)) ([441a35c](441a35c))
* design: dark mode adjustments ([#1091](#1091)) ([9c67a4a](9c67a4a))
* design: Footer styling refactor for accessibility and responsive design improvements ([#1081](#1081)) ([18dd17f](18dd17f))
* local personnel-api service targets builder stage only ([#1084](#1084)) ([94950eb](94950eb))
* storybook dark theme ([#1065](#1065)) ([c5deda3](c5deda3))
* storybook deploy ([#1072](#1072)) ([ee21044](ee21044))


### Security Improvements

* **deps:** update auto-instrumentations-web and auto-instrumentations-node ([#1087](#1087)) ([71236b8](71236b8))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shkeating shkeating shkeating approved these changes

@gidjin gidjin gidjin approved these changes

@abbyoung abbyoung Awaiting requested review from abbyoung

@jbecker01 jbecker01 Awaiting requested review from jbecker01

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jcbcapps @shkeating @gidjin @abbyoung