Disable request diagnostics modal on install #5306
Conversation
markmhendrickson
force-pushed
the
feat/remove-request-diagnostics
branch
from
59fade6 to
be4bb75
Compare
| @@ -12,7 +12,7 @@ function isHomePage(pathname: RouteUrls) { | |||
| } | |||
|
|
|||
| export function isLandingPage(pathname: RouteUrls) { | |||
| return pathname === RouteUrls.RequestDiagnostics || pathname.match(RouteUrls.Onboarding); // need to match get-started/ledger | |||
| return pathname === pathname.match(RouteUrls.Onboarding); // need to match get-started/ledger | |||
There was a problem hiding this comment.
| return pathname === pathname.match(RouteUrls.Onboarding); // need to match get-started/ledger | |
| return pathname.match(RouteUrls.Onboarding); // need to match get-started/ledger |
I can clean this up further as I will refactor this area
| @@ -129,7 +129,6 @@ function useAppRoutes() { | |||
| </OnboardingGate> | |||
| } | |||
| > | |||
| <Route path={RouteUrls.RequestDiagnostics} element={<AllowDiagnosticsModal />} /> | |||
There was a problem hiding this comment.
You need to remove the import of AllowDiagnosticsModal and also delete that file @app/pages/onboarding/allow-diagnostics/allow-diagnostics'
|
There's some valid test failures we will need to fix. I will help with that soon |
pete-watters
force-pushed
the
feat/remove-request-diagnostics
branch
from
072b2c3 to
07827c2
Compare
|
I did some refactoring here and got the tests to pass. We should get another review before squashing commits then approving and merging. |
There was a problem hiding this comment.
+1 to rebasing. @markmhendrickson when updating a PR, good practice to use git commit --amend flag to not create unnecessary commits.
Otherwise, LGTM. Love to see more lines of code removed than added.
| @@ -14,7 +14,7 @@ export interface DialogProps { | |||
| interface RadixDialogProps extends DialogProps { | |||
| children: ReactNode; | |||
| footer?: ReactNode; | |||
| header?: ReactElement<any, string | JSXElementConstructor<any>>; | |||
| header: ReactElement<any, string | JSXElementConstructor<any>>; | |||
There was a problem hiding this comment.
Not a change in this PR but, why not ReactNode here 🤔
There was a problem hiding this comment.
I will change it separately👍 . Not sure why I didn't do that in the first place
pete-watters
force-pushed
the
feat/remove-request-diagnostics
branch
from
07827c2 to
e6cd821
Compare
This PR removes the "Help is improve" dialog from display upon installation of the extension. This change is intended to reduce friction during onboarding.
We may add the ability to disable pseudonymous analytics later inside of the app's general settings and outside of the onboarding flow.
Before
After