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

Sort tabs with less steps #2

Open
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
1 participant
@piroor
Copy link

commented Apr 18, 2019

Hello,

Currently this addon tries to move all tabs always, even if they don't need to be moved. This approach may cause some troubles:

  • It may produce too much tab moves. When there are too many tabs (for example, over 1000+) it may take much time.
  • It may scramble tabs unexpectedly and unnecessarily. It is critically dangerous on some kinds of addons (like Tree Style Tab).

So, instead only minimum tabs should be moved based on the algorithm of "diff". This PR contains a ported version of diff and changes to move tabs based on the calculated difference. The main logic is ported from Tree Style Tab, and this PR is based on my another PR for Firefox Multi-Account Containers.

How about this change?

piroor added some commits Apr 18, 2019

@piroor

This comment has been minimized.

Copy link
Author

commented Apr 18, 2019

Here are some additional information for reviewing:

  • The largest addition differ.js is a ported version from the diff module of npm. I just modified it to remove needless codes for this usecase.
  • The differ detects three type differences: "added", "removed", and "unchanged". Rearranging of tabs is based on a logic:
    1. Prepare three arrays: initial order (beforeIds), final order (afterIds), and the current order (currentIds: this is required because tabs.move() is asynchronous and we need to track the order of tabs after tab moves.)
    2. Calculate differences between the initial order and the final order.
    3. Extract only "added" differences. Any "removed" difference should be ignored, because those tabs are moved with corresponding "addded" differences. Of course there is no removed tab while while rearranging.
    4. Calculate destination position of "added" tabs, based on the current order (currentIds).
    5. Move "added" tabs actually via tabs.move().
    6. Update the current order (currentIds) for the next calculation.
    7. Repeat steps 4-7 for all "added" differences.
@piroor

This comment has been minimized.

Copy link
Author

commented Apr 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.