Skip to content

Conversation

@georgeweiler
Copy link
Contributor

This PR builds the suggestion page according to designs: https://www.figma.com/file/Tx1lZc7us4SqCeAgbt1hBV/Threshold-Dapp?node-id=4251%3A276860&t=ZmBL64H8ytLsbgVf-0

Per UX discussion we've decided to implement an override for posthog capture events so that suggestions can still be submitted to posthog, even if the user has declined the anonymous analytics tracking.

User files are uploaded to the Google cloud bucket which are permission locked for authorized users only. The link to the file is sent to posthog.

@github-actions
Copy link

Copy link
Contributor

@r-czajkowski r-czajkowski left a comment

Choose a reason for hiding this comment

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

Left minor comments. Overall LGTM 👍

Comment on lines +12 to +17
return fetch(`https://storage.googleapis.com/${bucket}`, {
method: "POST",
body: formData,
}).then((resp) => {
return `https://storage.cloud.google.com/${bucket}/${file.name}`
})
Copy link
Contributor

Choose a reason for hiding this comment

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

We already use axios as a http client see utils/exchangeApi.ts- we could use it, probably simplify code a little bit. Ofc non-blocking.

return fetch(`https://storage.googleapis.com/${bucket}`, {
method: "POST",
body: formData,
}).then((resp) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe let's add catch to log any errors and avoid app crashes.

export const useCapture = (eventName: PosthogEvent) => {
export const useCapture = (
eventName: PosthogEvent,
overrideConsent = false
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe shouldOverrideConsent?

suggestion: suggestionText,
})

setIsLoading(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably should set the loading indicator to false once we catch an error.

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