Skip to content
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

Add pendo and user information #51

Merged
merged 5 commits into from
Oct 20, 2018
Merged

Conversation

ryelo
Copy link
Member

@ryelo ryelo commented Oct 16, 2018

No description provided.

Copy link
Contributor

@karelhala karelhala left a comment

Choose a reason for hiding this comment

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

Looks good, I have just one small concern about loading chrome from pug.

@@ -1,3 +1,7 @@
script(src='/insights/static/chrome/js/chrome.js')
script.
window.getName();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you call this function from chrome instead of HTML? I am kinda worried that the chrome file will get loaded twice.

src/js/chrome.js Outdated
import jwt from 'jwt-redhat';

const onAuth = auth();

onAuth.then(() => {
analytics(jwt.getUserInfo());
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you call the getName function from here? So the data will be loaded emediately when user logs in.

@ryelo
Copy link
Member Author

ryelo commented Oct 17, 2018

@karelhala I abstracted some of the stuff, so take another look and tell me what you think. Not sure if abstracting the onAuth inside the getName() function is okay since it gets called within onAuth in the beginning of the file. If it's needed, I can add it back into the function.

@karelhala karelhala merged commit ec886a4 into master Oct 20, 2018
@ryelo ryelo deleted the enhancements/pendo-with-user-info branch March 1, 2019 22:20
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.

2 participants