search

USACE / groundwork

React Components for USACE Applications
https://usace.github.io/groundwork
4 stars 0 forks source link

NWPS Hooks #49

Closed jbkolze closed 2 months ago

jbkolze commented 2 months ago

Added a couple hooks for the NWPS API. Might be nearing a point soon where we'll need to think about additional nesting for WM components, splitting out hooks, etc.

Resolves #47

adamscarberry commented 2 months ago

Not a technical comment, but more of a consideration when publishing other agencies data.

It's one thing to have reference data like flood stage values displayed that come from an external agency, but publishing their forecasts on our websites should be used with care. Proper attribution/credit and linking back to their site for such products may be a best practice.

When we used to link or display their hydrographs (with forecasts), it was more apparent that we were using their products. We just want to communicate to public users that certain gage forecasts were not generated by USACE.

In other cases, we will be publishing our own forecasts for reservoirs.

jbkolze commented 2 months ago

@adamscarberry Fully agree there. I think a note in this regard (e.g. make sure clear labeling / a disclaimer is used) should be included in the groundwork docs, but I'm unsure where would be most appropriate. Highest visibility would probably be to add a note to every component / hook that uses data from an outside agency. And layout/display components could potentially include a prop(s) for proper labeling/messaging (by default?).

adamscarberry commented 2 months ago

@jbkolze Yea it definitely makes sense to bake into component. Maybe for useNwpsGaugeData a very prominent notice near the top of the page?

I just don't want USACE pages to turn into repackaged NWS data because folks want to see certain view for internal use.

jbkolze commented 2 months ago

@adamscarberry I think it might be difficult to tie the notice into the hook itself. There's no telling where / how the data is used, so a general notice at the top would have to be something along the lines of "There's some NWS data on this page somewhere" which in my opinion doesn't really solve the problem. I do think it makes sense to do that in display components that use the hook since you can relate the notice to the data/component spatially.

All that being said, stepping back a little... Is this a can of worms we just don't want to open in the first place? I'm not married to the idea -- I originally thought displaying downstream / control point conditions for projects would be useful, but, like you said, at the end of the day that's probably much more of an internal concern. And the same general thing can be accomplished just by linking to the NWS -- just a little clunkier from a UX perspective.

adamscarberry commented 2 months ago

@jbkolze Sorry for the confusion, I meant a a very prominent notice near the top of the page on the docs page.

I see value in getting the NWS forecast data to perhaps add into an existing plot where we have our own observed data. After all we (USACE) are not the agency for stream gage forecasts, only reservoirs.

Maybe we stop here with the hooks and don't make components that display other agency data.

@willbreitkreutz, you have any thoughts on this?

willbreitkreutz commented 2 months ago

@adamscarberry @jbkolze I think it makes sense for us to build hooks/components for this kind of data, if we don't do it so others follow our lead, then they will just do it themselves in haphazard ways. This comes with a couple considerations though:

  1. We need to come up with some disclaimer text that can be used by a consumer of the data/component.
  2. We could add a component that is an orange <TiInfo /> or other icon that pops up the disclaimer when clicked.
  3. We require that when using the hook/display component you must use the disclaimer text or disclaimer component.
  4. I wouldn't try and programmatically enforce this, but the note in the docs should say that we reserve the right to break their sites if they don't follow our usage guidelines.
willbreitkreutz commented 2 months ago

@jbkolze looks like merging the other cda timeseries hook PR caused some conflicts here, want to take a look or I can.

jbkolze commented 2 months ago

@willbreitkreutz I'll take a look at it.

jbkolze commented 2 months ago

@willbreitkreutz @adamscarberry This should be good to go now from a merging perspective. Do we want to hold off until we have the disclaimer text/component available, though? Will go add some thoughts/questions in that issue.

willbreitkreutz commented 2 months ago

@jbkolze i'm good with merging and adding the disclaimer as we go forward since we're still in YOLO mode.