Replace loading text with spinner in RunDetails page

[RATNAPRADEEP]

Description

Replaces the plain loading text in the RunDetails page with a Material-UI CircularProgress spinner to improve user experience while data is loading.

Type of Change

• Frontend (UI improvement)

Notes

• No functional logic changes
• No backend changes
• No lockfile modifications

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign chensun for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• frontend/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[github-actions]

🎉 Welcome to the Kubeflow Pipelines repo! 🎉

Thanks for opening your first PR! We're excited to have you onboard 🚀

Next steps:

• Check out the Contributing Guide for repo-specific guidelines and the Kubeflow Contributor Guide for general community standards
• Join our #kubeflow-pipelines Slack channel
• Attend the Kubeflow Pipelines WG meetings

Feel free to ask questions in the comments.

[google-oss-prow]

Hi @RATNAPRADEEP. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[Copilot]

Pull request overview

This PR updates the RunDetails page loading experience by replacing the plain loading text with a centered Material-UI CircularProgress spinner and keeping a loading message beneath it.

Changes:

• Replace the isLoading render branch with a centered spinner + “Currently loading run information...” text.
• Includes a number of formatting-only indentation changes elsewhere in RunDetails.tsx (no functional behavior changes observed).

[Copilot]

The nested ternary blocks here (and other places in this PR) appear to be re-indented without changing behavior. Since this PR’s stated goal is only the loading spinner UI, consider reverting unrelated formatting-only changes to keep the diff focused and reduce future merge conflicts.

[Copilot]

The loading-state rendering has changed (spinner + updated message), but there doesn’t appear to be a unit test covering the isLoading path for this page. Since this file already has extensive tests, add a small test asserting the loading UI (e.g., presence of a progressbar and the loading text) to prevent regressions.

[jeffspahr]

/ok-to-test

[jeffspahr]

Code Review: Replace loading text with spinner in RunDetails

Critical Bug: isLoading state is never set to false

This PR will break the RunDetails page. The new isLoading state is initialized to true but is never set to false anywhere in the diff. The component will be permanently stuck on the loading spinner.

The original code used this.props.isLoading, which was driven by the parent RunDetailsRouter (RunDetailsRouter.tsx:73):

return <EnhancedRunDetails {...props} isLoading={runIsFetching || templateStrIsFetching} />;

That prop correctly transitions from true to false when data fetching completes. The new state-based approach has no corresponding setState({ isLoading: false }) call — none of the setStateSafe calls in the component include isLoading: false.

Additional Issues

1. Dead prop: The isLoading prop on RunDetailsInternalProps (line 104) is now unused but not removed. The parent still passes it, but it's ignored. This is misleading.

2. Inline styles: The PR uses inline styles (style={{ textAlign: 'center', paddingTop: '40px' }}). The file already uses typestyle for CSS (the css object). The new styles should use the same pattern for consistency.

3. No test changes: The RunDetailsInternalProps interface exports isLoading for testing. If the behavior changes from prop-driven to state-driven, tests should be updated to reflect this.

Recommendations

1. Don't move to state at all. The prop-driven approach is correct — the parent already manages loading state via React Query's isFetching. Just update the JSX to use the spinner while keeping this.props.isLoading:

   if (this.props.isLoading) {
     return (
       <div style={{ textAlign: 'center', paddingTop: '40px' }}>
         <CircularProgress />
         <div>Currently loading run information.</div>
       </div>
     );
   }

2. If state-based loading is truly desired, add isLoading: false to the setStateSafe call at the end of the load() method and in the catch block.

3. Move inline styles to the existing css stylesheet object.

4. Clean up the unused isLoading prop from RunDetailsInternalProps if switching to state.