Skip to content

Conversation

@brooketopcoder
Copy link
Contributor

@brooketopcoder brooketopcoder commented Nov 18, 2022

This PR adds the Uni Nav snippet and does some other linting and config fixes.

It also removes all the now obsolete header code.

A future PR will change the tool title dynamically in the header when the tool changes.

@brooketopcoder brooketopcoder self-assigned this Nov 18, 2022

useEffect(() => {

UniNavSnippet(EnvironmentConfig.UNIVERSAL_NAV.URL)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Better to move this into a separate useEffect with no dependencies. We don't want the script to run multiple times.

Better yet, you can call UniNavSnippet even before the Header declaration - there's no need to wait for react to run before we can initialize the script (basically we can let the script download and react do it's thing in parallel).

…v outside the scope of the Header component. #time 15m
Copy link
Collaborator

@vas3a vas3a left a comment

Choose a reason for hiding this comment

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

Looks great!

@brooketopcoder brooketopcoder merged commit 1f6f094 into PROD-3115_uni-nav Nov 21, 2022
@brooketopcoder brooketopcoder deleted the PROD-3232_integrate-nav branch November 21, 2022 20:10
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