Implement container reordering #1608

sryze · 2019-12-25T10:32:07Z

This is just a small patch that makes it possible to move / reorder containers via drag&drop.

I added a third button next to the edit (pencil) button that allows users to drag a container and insert it before another container. Container positions are saved in the browser's local storage.

I got the icon from the Mozilla Foundation Icon Font.

Related issue: #330

Example:

firefox_multi-account_containers-6.1.1.zip

lindhe

LGTM. I cherry-picked your commit, built it and tried it. Seems to work as intended.

npm run test seems to be flaky, but that goes for master too ("should uncancel after 2 seconds" fails with "Error: Timeout of 2002ms exceeded.").

src/img/container-move.svg

src/js/popup.js

jonathanKingston · 2020-01-21T11:40:38Z

src/js/popup.js

@@ -908,6 +922,53 @@ Logic.registerPanel(P_CONTAINERS_EDIT, {
      const tr = document.createElement("tr");
      fragment.appendChild(tr);
      tr.classList.add("container-panel-row");
+      tr.draggable = true;
+      tr.dataset.containerId = identity.cookieStoreId;
+      tr.addEventListener("dragstart", e => {


Suggested change

tr.addEventListener("dragstart", e => {

tr.addEventListener("dragstart", (e) => {

Just to remain consistent with other code in this repo

jonathanKingston · 2020-01-21T11:42:11Z

src/js/popup.js

+        if (droppedElement && droppedElement !== tr) {
+          tr.classList.remove("drag-over");
+          parent.insertBefore(droppedElement, tr);
+          const containerOrder = {};


This part from 961-969 of saving the order feels like it should be a separate function probably.

src/js/popup.js

jonathanKingston

This seems great to me!
Included a few comments, I would prefer we simplify the storage etc.
Would you be interested in implementing this also into firefox as a follow up? (I'm happy to help mentor this).

sryze · 2020-02-01T19:24:06Z

@jonathanKingston I fixed some stuff according to your suggestions, please take a look

Would you be interested in implementing this also into firefox as a follow up? (I'm happy to help mentor this).

I'm not sure, how does that work? do I create a PR in the Firefox repo and add you as a reviewer?

Laurensdc · 2020-02-14T09:27:40Z

This needs to happen

idoleat · 2020-03-14T07:30:44Z

I would be much appreciated if this PR is approved 🙏 Thanks for your awesome job!

jwdevel · 2020-04-10T02:41:24Z

Query: will the new order chosen here be reflected when the "sort tabs" button is clicked?

At least from my previous experimenting, changing the order in containers.json does not affect that other ordering, and I wonder if this is any different.

jrdnrch · 2020-05-15T14:55:12Z

Bump on this! It would certainly be very helpful. I'll even help out with testing if need be. Thanks for this. Hopefully it gets approved soon.

aghassemlouei · 2020-06-08T12:19:44Z

Any progress on approvals?

lindhe · 2020-06-09T18:53:29Z

@jonathanKingston What's the review process like? Can you approve this, or do we need someone else from Mozilla to look at it too?

MurzNN · 2020-06-10T04:42:15Z

Maybe @maxxcrawford can help with reviewing and merging, or comment situation? He do last merge recently.

nicolasbadia · 2020-06-17T17:12:25Z

Anyone to approve this? Can we help?

heitorPB · 2020-06-17T17:42:41Z

There are conflicts in src/css/popup.css and in src/js/popup.js.

@sryze, could you rebase to fix this?

gethubb · 2020-07-07T02:14:31Z

This looks great! Hope you're able to do the fix @sryze and get the approval... good work!

maxxcrawford

@sryze This will need to be caught up with the main branch, as there are new UX/UI revisions. Can you catch it up and confirm it works as expected in the new layout?

maxxcrawford · 2020-07-07T14:56:45Z

@MurzNN Thanks for the tag. Reviewed!

sryze · 2020-07-11T10:09:58Z

@madalincm I will try to fix the conflicts today.

urda · 2020-07-11T10:24:21Z

@madalincm I will try to fix the conflicts today.

Your efforts are appreciated

sryze · 2020-07-11T11:47:54Z

Merged with master, in the new version the "move" buttons will be shown inside the "Manage Containers" menu:

Also I couldn't get the top black border effect to work for some reason... so now containers are simply highlighted in gray when you drag over them.

lindhe · 2020-08-05T18:20:39Z

@maxxcrawford It says that you have requested changed, but I see no lingering comments. Perhaps you need to review again?

Makishima · 2020-08-14T15:06:40Z

Can we have this feature, please?

mahalel · 2020-08-26T20:31:36Z

Please can we have this feature, I have over 30 containers now and it's getting hard to find the right one when needed :(

lindhe · 2020-11-22T13:28:22Z

Since this is getting close to a year since publication, I would very much appreciate a statement from Mozilla explaining why this is not getting any traction. Is the feature unwanted or is it hard to merge for some technical reason? In the world of CI/CD, this seems to lack the C.

urda · 2021-01-01T08:34:24Z

What's the status of this PR? Can we get this feature in 2021?

maxxcrawford

@sryze This is ready to merge in, pending the dark mode we added today. Could you please resolve the conflicts one more time? After that, we can add this in! (But also, be sure this looks/works as expected in dark mode)

duoi · 2021-01-21T00:11:01Z

@maxxcrawford it's 2021, you get paid to do this, after Mozilla dragged their knuckles for a literal year why don't you just fix the conflicts and merge it in?

jc00ke · 2021-01-21T01:33:06Z

@duoi hey dude, that's super rude and uncalled for. A lot of us share in your frustration but that's no way to talk to someone in OSS.

groovecoder · 2021-01-21T17:51:04Z

Thank you @jc00ke.

@duoi - this project's Code of Conduct is the Mozilla Community Participation Guidelines which state:

frustration should never turn into a personal attack

Don't make statements that personally attack a member of this project, or you will be banned from participation.

sryze · 2021-01-21T18:02:03Z

@sryze This is ready to merge in, pending the dark mode we added today. Could you please resolve the conflicts one more time? After that, we can add this in! (But also, be sure this looks/works as expected in dark mode)

Done

duoi · 2021-01-21T19:31:46Z

@groovecoder didn't personally attack anybody, I literally just asked why he doesn’t fix the merge conflict himself. That’s not a personal attack any more than this comment is lol. Anyway, let’s not derail.

groovecoder

Code and spot-check looks good.

groovecoder · 2021-01-22T01:05:55Z

(I tested in dark mode.)

maxxcrawford

@sryze This is great. Merging in now. Thanks for all the hard work on this.

urda · 2021-02-02T00:08:26Z

When will this release to production?

nicolasbadia · 2021-02-02T00:17:52Z

@urda It already is released to production.

urda · 2021-02-02T00:41:32Z

@nicolasbadia Ah guess my add-ons haven't updated yet! Just got the latest and greatest and re-ordering my containers!

As promised, donation sent:

jc00ke · 2021-02-02T02:24:23Z

I'm so happy! Thank you all so much for making this happen. Was this issue open a long time? Sure. Do I give a sh.... not anymore! I really appreciate the push towards the end from those responsible for code review and getting this into production. 🙏 🙏 🙏

maxxcrawford · 2021-02-02T15:20:20Z

@urda If you add-ons are not set to update automatically, you can update them by going to about:addons and clicking on the gear icon and selecting "Check for updates"

placoderm · 2022-03-25T12:28:11Z

I am running 8.0.6 version of the plugin and I don't see the handles to re-arrange the containers.
Any idea what I am doing wrong? FF 98.0.1

Sorry if this isn't the right place to ask this question.

Vinnl · 2022-03-25T12:33:00Z

Hi @placoderm, your screenshot is from Firefox's settings, whereas this update affected the extension menu (install the extension from here if you don't have it yet). You can then re-order containers by opening the menu (pictured here) and clicking "Manage Containers" at the bottom.

placoderm · 2022-03-25T12:59:35Z

@Vinnl Thanks for the super fast reply.

I wasn't seeing that at the bottom of the list. I had to zoom waaaaay out to see it.

I have moved one of the containers to the top (the yellow sun glasses)

However, when I press and hold the + button to open a new container, the possition hasn't changed. You can see it still in its original place:

I have closed and restarted FF.

Any ideas?

achernyakevich-sc · 2022-03-25T17:38:20Z

@placoderm Be frankly I didn't know that holding + shows list of containers that you could open tab for. :)

But I can confirm that this is a bug... I rearranged my containers months ago and now I see that list in main menu and behind of + button sorted different way.

This thread is pull request related. I suspect it will not be visible for big audience. So I would propose to move it to separate issue where you could describe use case in details, provide screenshots and people could vote for fixing it. Here you could drop a comment with reference to the issue you will create.

sryze mentioned this pull request Dec 29, 2019

Add ability to reorganize Containers #330

Closed

lindhe approved these changes Dec 29, 2019

View reviewed changes

jonathanKingston reviewed Jan 21, 2020

View reviewed changes

src/img/container-move.svg Outdated Show resolved Hide resolved

jonathanKingston reviewed Jan 21, 2020

View reviewed changes

src/js/popup.js Outdated Show resolved Hide resolved

jonathanKingston reviewed Jan 21, 2020

View reviewed changes

src/js/popup.js Outdated Show resolved Hide resolved

jonathanKingston reviewed Jan 21, 2020

View reviewed changes

src/js/popup.js Outdated Show resolved Hide resolved

jonathanKingston suggested changes Jan 21, 2020

View reviewed changes

egemenu approved these changes Jan 30, 2020

View reviewed changes

maxxcrawford self-requested a review July 7, 2020 14:55

maxxcrawford requested changes Jul 7, 2020

View reviewed changes

maxxcrawford requested changes Jan 11, 2021

View reviewed changes

Implement container reordering

89719f7

groovecoder approved these changes Jan 22, 2021

View reviewed changes

maxxcrawford approved these changes Jan 22, 2021

View reviewed changes

groovecoder merged commit 68b1b2f into mozilla:master Jan 22, 2021

maxxcrawford mentioned this pull request Jan 22, 2021

Reorder Containers page #1879

Closed

This was referenced Jan 22, 2021

[Feature Request]: Option to re-arrange the order of containers displayed in the list #1804

Closed

Feature: Reordering containers #1843

Closed

Sorted Container List #1866

Open

7.0.X UX/UI Feedback #1784

Closed

This was referenced May 19, 2022

Feature Request - Organize Tabs in Dropdown #1639

Closed

How to manually re-arrange containers? #1951

Closed

dannycolin mentioned this pull request Jun 13, 2022

rearrange the containers #1720

Closed

	tr.addEventListener("dragstart", e => {
	tr.addEventListener("dragstart", (e) => {

Implement container reordering #1608

Implement container reordering #1608

Conversation

sryze commented Dec 25, 2019 • edited Loading

lindhe left a comment

Choose a reason for hiding this comment

jonathanKingston Jan 21, 2020

Choose a reason for hiding this comment

sryze Mar 27, 2020

Choose a reason for hiding this comment

jonathanKingston Jan 21, 2020

Choose a reason for hiding this comment

jonathanKingston left a comment

Choose a reason for hiding this comment

sryze commented Feb 1, 2020 • edited Loading

Laurensdc commented Feb 14, 2020

idoleat commented Mar 14, 2020

jwdevel commented Apr 10, 2020

jrdnrch commented May 15, 2020

aghassemlouei commented Jun 8, 2020

lindhe commented Jun 9, 2020

MurzNN commented Jun 10, 2020

nicolasbadia commented Jun 17, 2020

heitorPB commented Jun 17, 2020

gethubb commented Jul 7, 2020

maxxcrawford left a comment

Choose a reason for hiding this comment

maxxcrawford commented Jul 7, 2020

sryze commented Jul 11, 2020

urda commented Jul 11, 2020

sryze commented Jul 11, 2020 • edited Loading

lindhe commented Aug 5, 2020

Makishima commented Aug 14, 2020

mahalel commented Aug 26, 2020

lindhe commented Nov 22, 2020

urda commented Jan 1, 2021

maxxcrawford left a comment • edited Loading

Choose a reason for hiding this comment

duoi commented Jan 21, 2021 • edited Loading

jc00ke commented Jan 21, 2021

groovecoder commented Jan 21, 2021

sryze commented Jan 21, 2021

duoi commented Jan 21, 2021

groovecoder left a comment

Choose a reason for hiding this comment

groovecoder commented Jan 22, 2021

maxxcrawford left a comment

Choose a reason for hiding this comment

urda commented Feb 2, 2021

nicolasbadia commented Feb 2, 2021

urda commented Feb 2, 2021

jc00ke commented Feb 2, 2021

maxxcrawford commented Feb 2, 2021

placoderm commented Mar 25, 2022

Vinnl commented Mar 25, 2022 • edited Loading

placoderm commented Mar 25, 2022

achernyakevich-sc commented Mar 25, 2022

sryze commented Dec 25, 2019 •

edited

Loading

sryze commented Feb 1, 2020 •

edited

Loading

sryze commented Jul 11, 2020 •

edited

Loading

maxxcrawford left a comment •

edited

Loading

duoi commented Jan 21, 2021 •

edited

Loading

Vinnl commented Mar 25, 2022 •

edited

Loading