Skip to content

Update map app workflow UI - #227

Open
pratikshac15 wants to merge 8 commits into
CIROH-UA:mainfrom
pratikshac15:update-map-app-workflow-ui
Open

Update map app workflow UI#227
pratikshac15 wants to merge 8 commits into
CIROH-UA:mainfrom
pratikshac15:update-map-app-workflow-ui

Conversation

@pratikshac15

Copy link
Copy Markdown
Contributor

Summary

This PR updates the NGIAB map application workflow (as per Issue #216) to provide a more complete and intuitive interface for running preprocessing and optional NextGen simulations directly from the map.

The workflow has been reorganized to guide users through selecting an input, configuring the simulation, and running the workflow from a single interface, while moving manual steps into a collapsible section below.

Changes

Complete Workflow

Added a unified workflow interface that allows users to:

  • Configure the date and time range.
  • Select either a gage or catchment as the input.
  • Specify an output directory.
  • Select the forcing dataset (NWM or AORC).
  • Choose the hydrologic model.
  • Optionally run the NextGen simulation after preprocessing.
  • Execute the entire workflow from a single Run Workflow action.

This update brings key NGIAB workflow options into the map application, enabling configuration and execution of workflows directly through the UI rather than the command line.

Note

This PR focuses on improving the map application workflow and exposing existing CLI functionality through the UI. No changes were made to the underlying preprocessing or model execution logic.

@pratikshac15

Copy link
Copy Markdown
Contributor Author

@quinnylee Issue #216

@chp2001 chp2001 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is really cool. Very nice quality of life and streamlining improvement.

I tested it with a few of the model types, and the feedback was somewhat lacking beyond the "complete" identifier, so I'm not completely confident it's doing everything properly, but it runs without errors and seems to do fine.

Nothing really wrong on the code side of things. Some scattered places where there's some refactoring that doesn't change functionality, but for the most part they seem to be improvements.

My primary complaint on this is that you removed some comments, and didn't seem to add new ones to explain functionality. Not the biggest deal, but it is bad for readability in my opinion.

Should be good to merge if no one else finds something wrong.

@pratikshac15

pratikshac15 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Thank you, Chad @chp2001, for taking the time to test it and for the detailed feedback.

I've addressed the readability comment by adding comments back around to make it easier to follow.

For the workflow feedback, just to make sure I understand correctly, are you referring to the fact that after trying different models, the UI only shows a generic "completed" message, so it's hard to verify that the selected options (such as the model or forcing dataset) were actually applied?
e.g.
✅ Preprocessing and NextGen simulation completed successfully.
Model: SUMMA
Forcing dataset: AORC
Output: ~/ngiab_output/gage-01646500

If that's what you mean, I agree that providing more detailed feedback after the workflow completes would improve the user experience, and I'd be happy to work on that as a follow-up improvement.

Thanks again for the review. :)

@quinnylee quinnylee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Really neat work Pratiksha! Thank you! The layout looks great as well. Once the bugs are fixed this should be good to go.

Comment thread modules/map_app/static/js/data_processing.js
Comment thread modules/map_app/static/js/data_processing.js
Comment thread modules/map_app/static/js/data_processing.js
Comment thread modules/map_app/static/js/main.js
Comment thread modules/map_app/static/js/main.js
@pratikshac15

Copy link
Copy Markdown
Contributor Author

Thanks for the review, Quinn @quinnylee . I addressed the requested changes:

  • Fixed the displayed output path so it matches the actual output folder for both catchment and gage workflows.
  • Updated workflow error handling so invalid catchment/gage IDs now show a workflow failure instead of a successful completion message.
  • Restored access to the USGS monitoring location from the gage popup while keeping the new workflow selection behavior.
  • Fixed the Ruff unused import issue.

I tested these locally with valid basin/gage inputs, an invalid basin/gage ID, and the gage popup link.

@quinnylee
quinnylee self-requested a review June 29, 2026 16:05

@quinnylee quinnylee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the changes! Just one thing, and this is my mistake, the actual default output directory is ~/ngiab_preprocess_output.

Really like the new gage tooltip

@pratikshac15

pratikshac15 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Sorry, my mistake. I didn't realize the CLI was using the configured output directory and assumed the default was always ~/ngiab_data_preprocess.

I've updated it to use the output directory resolved by FilePaths, so the path shown in the UI now matches the actual location being used when no output directory is specified.

Also, I've run Ruff formatting locally and pushed the formatting changes.
@quinnylee

@quinnylee
quinnylee self-requested a review June 29, 2026 18:09

@quinnylee quinnylee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great work @pratikshac15! Ruff formatting is not actually required for merge but still a nice touch. Thanks for all the work!

@pratikshac15

Copy link
Copy Markdown
Contributor Author

Hey @quinnylee, are we merging this PR?

@quinnylee

Copy link
Copy Markdown
Contributor

Hey @quinnylee, are we merging this PR?

@JoshCu your call

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.

3 participants