Skip to content
This repository has been archived by the owner on Oct 22, 2024. It is now read-only.

feat: ask user confirmation on unpair #537

Merged

Conversation

OGPoyraz
Copy link
Member

Overview

This PR implements a confirmation dialog when the user tries to unpair while paired.

@OGPoyraz OGPoyraz requested a review from a team as a code owner February 20, 2023 10:38
@OGPoyraz OGPoyraz linked an issue Feb 20, 2023 that may be closed by this pull request
@OGPoyraz OGPoyraz force-pushed the feat/403-ask-user-why-they-are-unpairing-the-desktop-app branch from 6f39757 to 199d0c9 Compare February 22, 2023 09:01
racitores and others added 4 commits February 22, 2023 13:46
* extension update

* chore: update ci and lavamoat
* chore: electron upgrade

* fix: use node-fetch instead of node 18 fetch

* fix: restore extension commit

* fix: restore commitid submodule

---------

Co-authored-by: Matthew Walsh <[email protected]>
* chore: update submodule

* fix: restore submodule commitid

* chore: submodule
@OGPoyraz OGPoyraz force-pushed the feat/403-ask-user-why-they-are-unpairing-the-desktop-app branch from 752ee4c to 04ef013 Compare February 22, 2023 12:46
})
.then(async (value) => {
if (value.response === 0) {
this.metricsService.track(EVENT_NAMES.DESKTOP_APP_UNPAIRED, {
Copy link
Member

Choose a reason for hiding this comment

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

Do we not want this inside the disable method so it's done at the source?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question, it actually depends if we want to capture this event regardless of the answer for this confirmation.
I will ask @vinistevam

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeap makes sense to move it to disable.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@OGPoyraz OGPoyraz force-pushed the feat/403-ask-user-why-they-are-unpairing-the-desktop-app branch from e73e082 to 4ee33fe Compare February 23, 2023 13:40
Copy link
Contributor

@vinistevam vinistevam left a comment

Choose a reason for hiding this comment

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

Great PR 🚀

@OGPoyraz OGPoyraz force-pushed the feat/403-ask-user-why-they-are-unpairing-the-desktop-app branch from b542369 to 8477307 Compare February 24, 2023 09:07
@OGPoyraz OGPoyraz force-pushed the feat/403-ask-user-why-they-are-unpairing-the-desktop-app branch from 01eecbe to 5cb0ad4 Compare February 24, 2023 09:24
@OGPoyraz OGPoyraz merged commit a3e2435 into main Feb 24, 2023
@cryptotavares cryptotavares mentioned this pull request Mar 1, 2023
@abdulmenan10
Copy link

Hf6

Copy link

CLA Signature Action:

Thank you for your submission, we really appreciate it. We ask that you all read and sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just by adding a comment to this pull request with this exact sentence:

I have read the CLA Document and I hereby sign the CLA

By commenting with the above message you are agreeing to the terms of the CLA. Your account will be recorded as agreeing to our CLA so you don't need to sign it again for future contributions to this repository.

1 out of 3 committers have signed the CLA.
@vinistevam
@racitores
@bergarces

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ask user why they are unpairing the desktop app
7 participants