-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
feat(3765): Add modal to include metric id before redirecting to support page #30415
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
}; | ||
|
||
export const DefaultStory = () => ( | ||
<VisitSupportDataConsentModal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
77c1d55
to
ed2bb2f
Compare
Builds ready [ed2bb2f]
Page Load Metrics (1670 ± 48 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
ed2bb2f
to
a9f2ac2
Compare
Builds ready [a9f2ac2]
Page Load Metrics (1747 ± 147 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
a9f2ac2
to
cfdf344
Compare
Builds ready [cfdf344]
Page Load Metrics (1759 ± 99 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
cfdf344
to
52a76bb
Compare
Builds ready [52a76bb]
Page Load Metrics (1715 ± 110 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
52a76bb
to
6f25c62
Compare
Builds ready [6f25c62]
Page Load Metrics (2144 ± 138 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
6f25c62
to
e24341c
Compare
Builds ready [e24341c]
Page Load Metrics (1810 ± 69 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
e24341c
to
a83dc21
Compare
Builds ready [a83dc21]
Page Load Metrics (1749 ± 60 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
||
export const DefaultStory = () => ( | ||
<VisitSupportDataConsentModal | ||
version="1.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line is not necessary because we're reading from the env var inside the component
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch ! 🕵🏻♂️ 915721b
category: MetaMetricsEventCategory.Settings, | ||
event: MetaMetricsEventName.SupportLinkClicked, | ||
properties: { | ||
url: supportLinkWithUserId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we could make this data more useful / easier to parse on the dashboards. We could pass the version directly as a property for example, for statistics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for suggestions, passed in as param now 915721b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant something like
// ...
properties: {
url: supportLinkWithUserId,
version
}
so PMs can see which versions redirected the most to the support page
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh the properties was defined before in this event, i didn't modify the logic, i'm worried it could cause inconsistency of previous events and the new ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fair point, let's leave it as is then
ui/components/app/modals/visit-support-data-consent-modal/visit-support-data-consent-modal.tsx
Show resolved
Hide resolved
Builds ready [d603064]
Page Load Metrics (1690 ± 54 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
b2e482d
to
f274bae
Compare
f274bae
to
31b2ba4
Compare
Builds ready [cfea884]
Page Load Metrics (1463 ± 50 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [460da69]
Page Load Metrics (1440 ± 49 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! thanks for fixing the issue I mentioned
@itsyoboieltr thank for spotting as well 🕵🏻♂️ ! |
Description
We currently redirect users to this url when they reach out to support: https://support.metamask.io/
We should update Extension client in order to redirect users to this url when they reach out to support: https://support.metamask.io/?metamask_version=11.16.12&metamask_profile_id=d9b2d63d-a233-4d4d-bd4b-5b3d5a6e2c5d&metamask_metametrics_id=f3b9c1d2-4a5e-4b6f-9e2d-8c4f7a1b3c6d
Passing MetaMask Identifier and the version of the app to support can help facilitate debugging on our side.
This PR added 2 entry points for this feature:
Related issues
Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3765
Manual testing steps
https://support.metamask.io/
Screenshots/Recordings
Before
After
https://www.loom.com/share/9a45da1640104488abb2c7b7000dbdc9?sid=069eee12-f244-44b6-8947-c213208570364
Pre-merge author checklist
Pre-merge reviewer checklist