Conversation
Deployment results
Logs #24079038811 |
frontend/app/components/redesign/components/ToolsWalletAddress.tsx
Outdated
Show resolved
Hide resolved
frontend/app/components/redesign/components/ToolsWalletAddress.tsx
Outdated
Show resolved
Hide resolved
kjmitchelljr
left a comment
kjmitchelljr
left a comment
There was a problem hiding this comment.
Looks good, agree with the feedback Sid gave, but see those changes have already been added 👍🏿
| {UMAMI_HOST && UMAMI_WEBSITE_ID && ( | ||
| <script | ||
| defer | ||
| src={`${UMAMI_HOST}/script.js`} | ||
| data-website-id={UMAMI_WEBSITE_ID} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
nit: with this script injection here, do you think we should do any checks to make sure the URLs aren't malformed?
| const walletAddressInfo = await getWalletAddress(walletAddressUrl) | ||
| walletActions.setWalletAddressId(walletAddressInfo.id) | ||
| await connect() | ||
| trackEvent('wallet_connected') |
There was a problem hiding this comment.
nit: could be a later add, but thinking keeping the events in a separate file with a const map of event names as more events get added
| <script | ||
| defer | ||
| src={`${UMAMI_HOST}/script.js`} | ||
| data-website-id={UMAMI_WEBSITE_ID} |
There was a problem hiding this comment.
Should we add data-domains attribute too, so in production, we only track when on right domain (and not on PR previews/staging)?
There was a problem hiding this comment.
I noticed. web mon sets data-domains
Any env without the vars set - script tag isn't rendered at all, zero analytics (i.e. no data pollution in staging).
I think we're safe in our case. set vars in local dev and prod only
There was a problem hiding this comment.
When we set up these vars in GitHub (CI), they'll end up setting for both staging/previews and production.
There was a problem hiding this comment.
right, then we need to specify data-domain to localhost for dev and webmonetization.org prod. likely through another variable
package.json
Outdated
| "analytics:start": "pnpm --filter localenv-analytics start", | ||
| "analytics:stop": "pnpm --filter localenv-analytics stop" |
There was a problem hiding this comment.
These can be replaced with pnpm -C localenv/analytics start command. If you believe that additional scripts like these are more ergonomic, feel free to keep them.
3 participants
Summary
Closes #624
root.tsx, injected only whenUMAMI_HOSTandUMAMI_WEBSITE_IDare setlocalenv/umami/docker-compose.ymlEvent tracked as setup
wallet_connected/banner,/widget,/offerwallclick_card_toollinkNotes