Skip to content

PROD-3234 Add Spinner while Header Loads -> PROD-3115_uni-nav #423

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

Merged
merged 1 commit into from
Nov 21, 2022

Conversation

brooketopcoder
Copy link
Contributor

@brooketopcoder brooketopcoder commented Nov 21, 2022

  • spinner
  • clean-up
  • tool route

…the loading spinner until the header is ready. It adds the url to the tool landing page. #time 2h
import {
authUrlLogin,
authUrlLogout,
authUrlSignup,
LoadingSpinner,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is every tool and app that implements UniNav going to need to do this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@testflyjets every tool will need to handle the authentication part (login/logout/sign up, and "set user")

@@ -55,6 +56,8 @@ const Header: FC = () => {
signOut() { window.location.href = authUrlLogout },
signUp() { window.location.href = authUrlSignup() },
toolName: activeToolName,
toolRoute: activeToolRoute,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@brooketopcoder you'll have to add an effect on activeToolName to "update" the uniNav.toolName with it.
Same thing with the user - init the nav without it, and update the tool nav when profileReady is true (this also solves the missing login/logout buttons).

Copy link
Contributor Author

@brooketopcoder brooketopcoder Nov 21, 2022

Choose a reason for hiding this comment

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

@vas3a yep--figured that out. I've done that in a separate PR.

@brooketopcoder brooketopcoder merged commit ae88227 into PROD-3115_uni-nav Nov 21, 2022
@brooketopcoder brooketopcoder deleted the PROD-3234_dev-center branch November 21, 2022 22:57
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