Skip to content

Address Cee comments

Given the branches on branches status of this repo, I'm documenting Cee's requested edits here, and will open a new MR to address them, making the changes to the latest version of the code instead of earlier versions.

Pipeline

    • Note mapshaper dependency in _targets.R
    • Make sure cue for p1_latest_forecast_date is always
    • munge_streamflow() rename subset_streamflow to not be the same as the name of the fxn called to generate it
    • Get p1_sites based on sites in all forecast files
    • munge_raw_forecast_data() move filter to before collect() to avoid pulling in data that's not needed
    • identify_streamflow_droughts() - add comment re: why using +1 for the end of drought
    • compute_drought_records() - be explicit about which site and files get used in each iteration of the map. This could be done using map2 which allows multiple inputs or a tibble of inputs with tibble(site, streamflow_csv, tresholds... and use pmap_dfr() over rows to be explicit
    • generate_site_map - increase size of point
    • Add function documentation throughout
    • when dir.create called, check if exists first

General Vue code

    • Use css var for all colors (see ExpandingLegend.vue)

MapboxMap.vue

    • Make map a global variable instead of a ref
    • Define categorical drought data bins for global use (in global-data-store)
    • Zoom to state extents, not extent of points in state
    • Clean up unused conusButton code
    • use selectedExtent instead of stateClicked throughout
    • Impacts global-data-store, ExtentSummary.vue, too
    • Explore raising selected point by moving to highlight layer
    • Note: would this mess w/ the pop-up on mobile?
    • confirm all icons on map use same shade of grey
    • Try adjusting pop-up styling to add drop shadow to pop a bit more
    • See if there is a slightly darker version of basemap to make white points pop a bit more

StatePicker.vue

    • Scale up state map image slightly on expand
    • Make active state label bold in addition to selection fill
    • BONUS - have state map update to show selected state

ExpandingLegend.vue

    • legendDataBins prop should be type array
    • Use css vars for all colors

SiteSummary.vue

    • Modify composable reference for station name to {{ globalDataStore.selectedSiteInfo?.station_nm || '' }}
    • Consider if there is a way to adjust layout to get timeseries in view on initial click
    • Consider making drought context text same size as graph legend
    • On mobile, explore expanding down rather than up over map.

TimeSeriesGraph.vue

    • Consider whether data needs to be fetched in onBeforeMount() and in watch(selectedSite) - is it refetching the same data?
    • Consider moving legend beneath graph
    • In chart components, delete out/update old WDFN comments, e.g, in streamflow charts
    • Make 30 pct line slightly thicker stroke
    • Make cursor: pointer for observed diamond, medians, uncertainty bars, and summary points
    • Consider increasing stroke width of uncertainty boxes when the given week is selected

SidbarControl.vue

    • Consider just showing 'observed' OR 'forecast' instead of both, to avoid confusion about what is shown
    • Explore if tick marks could be hacked in.
    • Vuetify slider does allow ticks, but Vuetify is much larger import

DialogBox.vue

    • Make .dialog css responsive: width: auto; max-width: min(90vw, 75rem)

TitleDialog.vue

    • Drop title from learn more button - redundant.
    • Consider dropping tooltip, since info is in FAQs

FaqDialog.vue

    • Remove cursor: pointer from FAQ button in dialog header

CollapsibleAccordion.vue

    • Try adding a bit of box shadow to better differentiate from background.

VisualizationView.vue

    • Revisit instantiation of selectedWeek to see if could be done in global-data-store
    • Note: relates to map loading in MapboxMap.vue

global-data-store

    • Change dataWeeks computed value to be const dataWeeks = computed(() => dateInfoData.value?.map(d => d.f_w) ?? [])
    • Update selectedDate definition to use ?? instead of ||
    • for setting selectedExtent, check if ...router.currentRoute is needed in router.replace({ ...router.currentRoute, query: { extent: selectedExtent}})
    • Note: might be needed to retain mapbox map params?
    • Updated selectedSiteInfo definition to return siteInfo.value?.find(d => d.StaID == selectedSite.value) || null
    • Update inDrought definition to return !!(selectedSiteConditions.value && selectedSiteConditions.value.pd < 20);
    • Update droughtStatusNA definition to return selectedSiteConditions.value?.pd === 999 || false;

time-series-diamonds.js

    • Calculate key x variables ahead and store, instead of recomputing

css

  • base.css - consider using a variable for stroke width on different UI features

Text

    • Consider making text more scannable by shortening, using direct language, using bullets, using more visual hierarchy
    • Consider using 'chart' instead of graph
    • Consider shortening "To learn more about streamflow drought thresholds and streamflow percentiles, visit the Modeling streamflow drought website." to "learn more about streamflow drought thresholds and streamflow percentiles' throughout
    • Consider adding more images in the FAQs to provide more context for some of the questions and explanations
  • Graph explainer
      • Reduce repetitive language in descriptions of percentile thresholds (note - same language in final FAQ).
      • User higher weight font for subheadings
  • FAQs
      • How categories defined - move first text chunk to below ones about USDM
      • Why cfs - NTH - consider including image of site summary view w/ timeseries
Edited by Hayley Corson-Dosch