Skip to content

Conversation

@danilowoz
Copy link
Contributor

@danilowoz danilowoz commented Aug 13, 2021

Related to CSB-918

@danilowoz danilowoz requested review from alexnm and smhutch August 13, 2021 08:51
@danilowoz danilowoz self-assigned this Aug 13, 2021
@danilowoz danilowoz changed the title fix(dashboard beta): wrap the component and instead of the route fix(dashboard beta): feature flag wrapper over the component instead of the route Aug 13, 2021
Copy link
Contributor

@smhutch smhutch left a comment

Choose a reason for hiding this comment

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

Nice find 💯

* feature-flag query takes a few ms to load, and meanwhile the Route
* redirect the page because the route hasn't been registered yet
*/
component={isFeatureFlagBeta ? BetaRepositoriesPage : () => null}
Copy link
Contributor

Choose a reason for hiding this comment

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

Would something like this work to avoid the possibility of rendering a blank page?

Suggested change
component={isFeatureFlagBeta ? BetaRepositoriesPage : () => null}
component={isFeatureFlagBeta ? BetaRepositoriesPage : () => <Redirect to={dashboardUrls.home(activeTeam)} />}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would end up with the same behavior as before because the Redirect would be rendered anyway. A final and right solution would need some kind of loading state, but I think it's over-engineered for this beta version in a scenario that we control who has access to this.

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit d622bd3:

Sandbox Source
Notifications Test Configuration

@lbogdan
Copy link
Contributor

lbogdan commented Aug 13, 2021

Build for latest commit d622bd3 is at https://pr6040.build.csb.dev/s/new.

@danilowoz danilowoz merged commit 002efc4 into master Aug 13, 2021
@danilowoz danilowoz deleted the fix/beta-route branch August 13, 2021 09:16
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.

4 participants