fix: add error handling to stepper steps #680
Conversation
You can test this by going to the human assembly and see the GTF error. xref galaxyproject#679
|
could this be made a warning, and we let ppl go to galaxy without it? |
There was a problem hiding this comment.
Pull Request Overview
This PR adds error handling to stepper steps in the workflow configuration component. The changes address an issue where users would see loading indicators indefinitely when GTF file fetching fails.
- Adds error state management and display to stepper steps
- Implements utility functions for consistent step state calculation
- Updates stepper components to handle error states properly
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| stepUtils.ts | New utility functions for calculating step active and button disabled states |
| types.ts | Updates Step interface to make active property required |
| useLaunchGalaxy.ts | Adds error handling and returns error state in status |
| types.ts (UseLaunchGalaxy) | Adds error property to Status interface |
| stepError.tsx | New component for displaying error messages in steps |
| launchStep.tsx | Integrates error handling using new utilities and error component |
| types.ts (UseUCSCFiles) | Adds error and isLoading properties to hook interface |
| hook.ts (UseUCSCFiles) | Implements comprehensive error handling for UCSC files fetching |
| gtfStep.tsx | Updates to use error handling and conditional rendering based on error state |
| disabled={getButtonDisabledState( | ||
| status.disabled, | ||
| status.loading, | ||
| !!status.error |
There was a problem hiding this comment.
The !!status.error conversion is redundant since getButtonDisabledState already handles the truthy conversion with !!hasError internally.
| !!status.error | |
| status.error |
Copilot uses AI. Check for mistakes.
| )} | ||
| <Button | ||
| {...BUTTON_PROPS.PRIMARY_CONTAINED} | ||
| disabled={!value} | ||
| disabled={getButtonDisabledState(!value, false, !!error)} |
There was a problem hiding this comment.
The !!error conversion is redundant since getButtonDisabledState already handles the truthy conversion with !!hasError internally.
| disabled={getButtonDisabledState(!value, false, !!error)} | |
| disabled={getButtonDisabledState(!value, false, error)} |
Copilot uses AI. Check for mistakes.
## Description we prev had #680 merged. id like to iterate on that, so that the behavior for missing GTFs is no worse than it was pre-stepper. we also have outstanding issue #682 but i dont want to close that w this, bc this should be revisited. i see this as an interim measure until we have ux for this properly sorted. ## Related Issue related to #682
You can test this by going to the human assembly and see the GTF error. xref #679
While not super actionable at least we're not showing the loading indicator indefinitely.