-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Namespace redirects no longer error and are snappier #2106
Conversation
@@ -29,16 +31,17 @@ export class ArchivedWorkflowList extends BasePage<RouteComponentProps<any>, Sta | |||
} | |||
|
|||
private get namespace() { | |||
return this.props.match.params.namespace || ''; | |||
return this.state.namespace; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Source of truth for namespace is now react state
} | ||
|
||
private set namespace(namespace: string) { | ||
document.location.href = uiUrl('archived-workflows/' + namespace); | ||
this.setState({namespace}); | ||
history.pushState(null, '', uiUrl('cron-workflows/' + namespace)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This edits the URL without forcing a reload. This is cosmetic only, as the source of truth for the namespace is now on react state
} | ||
|
||
constructor(props: RouteComponentProps<any>, context: any) { | ||
super(props, context); | ||
this.state = {continue: ''}; | ||
this.state = {continue: '', loading: true, namespace: this.props.match.params.namespace || ''}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Desired namespace is loaded from URL only once
} | ||
|
||
public render() { | ||
if (this.state.loading) { | ||
return <Loading />; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Show a nice loading screen while API calls are being made
Codecov Report
@@ Coverage Diff @@
## master #2106 +/- ##
========================================
Coverage ? 8.89%
========================================
Files ? 61
Lines ? 34597
Branches ? 0
========================================
Hits ? 3079
Misses ? 31131
Partials ? 387 Continue to review full report at Codecov.
|
Fixes #2102
Demo (notice
managedNamespace
redirection still happens): https://cl.ly/4f3a073db983