Impls #1792 adds an ability to reopen current tab in the specific container with a shortcut #2721

antondudakov · 2025-02-21T12:24:59Z

Before submitting your pull request

I agree to license my code under the MPL 2.0 license.
I rebased my work on top of the main branch.
I ran npm test and all tests passed.
I added test coverages if relevant.

Description

Issue #1792

Type of change

Select all that apply.

Bug fix
New feature
Major change (fix or feature that would cause existing functionality to work differently than in the current version)

reopen.in.container.with.shortcut.mov

PR to localization: mozilla-l10n/multi-account-containers-l10n#31

antondudakov · 2025-04-20T10:38:10Z

Hey, guys.
Can I have a review, please?
@dannycolin
@groovecoder
@rafeerahman

antondudakov · 2025-07-14T12:51:00Z

Hey, guys.
Can I have a review, please?
@dannycolin
@groovecoder
@rafeerahman

😭😭😭
Can we have this?

antondudakov · 2025-07-14T13:00:45Z

Hey, guys.
Can I have a review, please?
@dannycolin
@groovecoder
@rafeerahman

😭😭😭 Can we have this?

@lesleyjanenorton

achernyakevich-sc · 2025-07-14T14:56:14Z

Hey, guys.
Can I have a review, please?
@dannycolin
@groovecoder
@rafeerahman

😭😭😭 Can we have this?

@lesleyjanenorton

@antondudakov this is open source and sometimes people just have no time to pay attentions because completely overloaded on their regular job.

What I could advise to increase chances for faster review and acceptance of PR:

implement only functionality that you would like to get: less code to review - easier to review and bigger chance to be reviewed.
if you would like to have code refactored - create separate PR: less code to review and with no new functionality - easier to review and bigger chance to be reviewed.
create a GitHub Issue that will describe what is the feature and how it will work, sometimes discussion in the issue result to completely different direction of feature implementation than initially it was expected.

antondudakov · 2025-07-14T16:11:51Z

Hey, guys.
Can I have a review, please?
@dannycolin
@groovecoder
@rafeerahman

😭😭😭 Can we have this?

@lesleyjanenorton

@antondudakov this is open source and sometimes people just have no time to pay attentions because completely overloaded on their regular job.

What I could advise to increase chances for faster review and acceptance of PR:
* implement only functionality that you would like to get: less code to review - easier to review and bigger chance to be reviewed.

* if you would like to have code refactored - create separate PR: less code to review and with no new functionality - easier to review and bigger chance to be reviewed.

* create a GitHub Issue that will describe what is the feature and how it will work, sometimes discussion in the issue result to completely different direction of feature implementation than initially it was expected.

This is exactly what I did.

The code that implement feature is just around 50 lines of code;
it's not a refactoring;
There's an issue Keyboard shortcut for "Reopen this site in" #1792, I also mentioned my PR in comments in the issue

.github/workflows/builds.yaml

src/js/background/backgroundLogic.js

antondudakov · 2025-08-01T06:18:39Z

Hey @achernyakevich-sc, I hope you're doing well.
Could you provide a review, please? I've addressed all your comments. 🙏🥹

antondudakov · 2025-08-25T09:15:33Z

@achernyakevich-sc, if you have a spare minute, please review. All addressed

dannycolin · 2025-08-29T22:10:11Z

src/manifest.json

+    },
+    "reopen_in_container_0": {
+      "suggested_key": {
+        "default": "Alt+Shift+1"


It would be best to let the default empty because conflicts with other addons or the browser are too common since there isn't a lot of key combos that are still free.

Thanks, will do

achernyakevich-sc · 2025-08-30T14:36:24Z

src/js/background/backgroundLogic.js

      if (command === "sort_tabs") {
        backgroundLogic.sortTabs();
        return;
      }

      for (let i=0; i < backgroundLogic.NUMBER_OF_KEYBOARD_SHORTCUTS; i++) {
        const key = "open_container_" + i;
+        const reopenKey = "reopen_in_container_" + i;
        const cookieStoreId = identityState.keyboardShortcut[key];
+
+        if (cookieStoreId === "none") {
+          continue;
+        }
+
        if (command === key) {
-          if (cookieStoreId === "none") return;
          browser.tabs.create({cookieStoreId});
+          return;
+        }
+
+        if (command === reopenKey) {
+          backgroundLogic.reopenInContainer(cookieStoreId);
+          return;
        }


I have returned to this piece of code third time and got in mind that it is quite hard for reading and understanding the logic. As in this PR @antondudakov add support of additional shortcuts then it would be reasonable as well to make small refactoring to make more transparent the shortcuts matching conditions and make it optimized as it is not possible at the same time initiate several shortcusts. :)

So I would propose to have flow controlling construction like:

if (command === "sort_tabs") { backgroundLogic.sortTabs(); } else if (command.startsWith("open_container_")) { for (let i=0; i < backgroundLogic.NUMBER_OF_KEYBOARD_SHORTCUTS; i++) { const key = "open_container_" + i; const cookieStoreId = identityState.keyboardShortcut[key]; if (command === key) { if (cookieStoreId !== "none") { browser.tabs.create({cookieStoreId}); } break; } } } else if (command.startsWith("reopen_in_container_")) { for (let i=0; i < backgroundLogic.NUMBER_OF_KEYBOARD_SHORTCUTS; i++) { const key = "reopen_in_container_" + i; const cookieStoreId = identityState.keyboardShortcut[key]; if (command === key) { if (cookieStoreId !== "none") { backgroundLogic.reopenInContainer(cookieStoreId); } break; } } }

I didn't check this code but I hope you got my idea.

But finally to make it working it looks we need to change loadKeyboardShortcuts() function in identityState.js.

Some another changes I would propose:

at the moment reopenInContainer() function is not used anywhere so maybe we could move its functionality back to the corresponding else if. At the moment there is no good reason for extraction.

I see at least three places where string literals like open_container_ and reopen_in_container_ will be used so maybe it would be reasonable to place it somewhere as constant. But maybe not for this PR but for some separated.

@antondudakov if you agree, could you update PR's code correspondingly.

Thanks, will do. I'll have some time next week

antondudakov · 2025-10-01T12:49:54Z

Hey, @bakulf, can you give feedback on this PR as well? I would really appreciate it.

…a shortcut

async from the `init()`

antondudakov mentioned this pull request Feb 21, 2025

adds copy for the name of a shortcut to reopen current tab in a specific container mozilla-l10n/multi-account-containers-l10n#31

Open

antondudakov mentioned this pull request Mar 6, 2025

Keyboard shortcut for "Reopen this site in" #1792

Open

achernyakevich-sc suggested changes Jul 14, 2025

View reviewed changes

.github/workflows/builds.yaml Show resolved Hide resolved

src/js/background/backgroundLogic.js Outdated Show resolved Hide resolved

src/js/background/backgroundLogic.js Outdated Show resolved Hide resolved

antondudakov force-pushed the antondudakov/reopen-shortcuts branch 2 times, most recently from 2647db2 to 81ee998 Compare July 15, 2025 06:59

antondudakov requested a review from achernyakevich-sc July 15, 2025 07:01

antondudakov changed the title ~~adds an ability to reopen current tab in the specific container with a shortcut~~ Impls #1792 adds an ability to reopen current tab in the specific container with a shortcut Aug 25, 2025

dannycolin reviewed Aug 29, 2025

View reviewed changes

achernyakevich-sc reviewed Aug 30, 2025

View reviewed changes

Anton Dudakov and others added 4 commits October 1, 2025 18:47

adds an ability to reopen current tab in the specific container with …

fd12eaa

…a shortcut

extracted reopenInContainer function which made it possible to remove

5775fdc

async from the `init()`

fixes merge conflicts

b77f8ff

address feedback

beb4a68

antondudakov force-pushed the antondudakov/reopen-shortcuts branch from 04d52b7 to beb4a68 Compare October 1, 2025 14:48

antondudakov requested review from achernyakevich-sc and dannycolin October 1, 2025 14:49

Impls #1792 adds an ability to reopen current tab in the specific container with a shortcut #2721

Are you sure you want to change the base?

Impls #1792 adds an ability to reopen current tab in the specific container with a shortcut #2721

Uh oh!

Conversation

antondudakov commented Feb 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Uh oh!

antondudakov commented Apr 20, 2025

Uh oh!

antondudakov commented Jul 14, 2025

Uh oh!

antondudakov commented Jul 14, 2025

Uh oh!

achernyakevich-sc commented Jul 14, 2025

Uh oh!

antondudakov commented Jul 14, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

antondudakov commented Aug 1, 2025

Uh oh!

antondudakov commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dannycolin Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

antondudakov Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

antondudakov Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

achernyakevich-sc Aug 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antondudakov Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

antondudakov Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

antondudakov commented Oct 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

antondudakov commented Feb 21, 2025 •

edited

Loading

antondudakov commented Aug 25, 2025 •

edited

Loading

achernyakevich-sc Aug 30, 2025 •

edited

Loading