Skip to content
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

Don't check for existing session tab in Chrome #2 #4

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

siipimutteri
Copy link

Removed checking of already existing session tab in Chrome. Implementation was buggy, when user closed tabs or sessions were expired.

This needs to be fixed, but as this version is using soon deprecating manifest v2, let's fix this when we upgrade to manifest 3.

One test prevented building, so I did a quick dirty fix and commented those lines out.

This hopefully fixes issue #2

Copy link
Contributor

@andreacavagna01 andreacavagna01 left a comment

Choose a reason for hiding this comment

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

The review started, with the removal of this check we are solving what issue?

Thabks for the PR Sami!

} else {
this.openSessionTab(sessionId, leappPayload);
}
this.openSessionTab(sessionId, leappPayload);
Copy link
Contributor

Choose a reason for hiding this comment

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

In this way, leapp will always open a new tab on Chrome?

@@ -1,6 +1,6 @@
{
"name": "leapp-browser-extension",
"version": "0.1.4",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that a patch needs to be created, sort of: 0.1.5 and not 0.1.4.1

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