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

Mouse drag and drop not working #21

Closed
117649 opened this issue Jan 25, 2021 · 25 comments
Closed

Mouse drag and drop not working #21

117649 opened this issue Jan 25, 2021 · 25 comments

Comments

@117649
Copy link
Contributor

117649 commented Jan 25, 2021

fb6614b
These changes will put DnD in to work but nothing seems right.

@onemen
Copy link
Owner

onemen commented Jan 25, 2021

Thank you @117649 , i will take a look one after finish working on multi-row

report i you see any errors in the browser console

@117649
Copy link
Contributor Author

117649 commented Jan 26, 2021

Not seeing any error in console so far.
It look like the codes work as intended but the changes in Firefox cause them generate unwanted behavior.
And also some of the changecode() may not in fully effective since the original function they are tap into have changed dramatically.

@onemen
Copy link
Owner

onemen commented Jan 26, 2021

Try to describe the visual bugs you see, what exactly doesn't work and how to reproduce each one

@117649
Copy link
Contributor Author

117649 commented Jan 26, 2021

Try to describe the visual bugs you see, what exactly doesn't work and how to reproduce each one

  1. Drop indicator: built-in style not working. Native indicator shows all the time and won't disappear. There is very limit time the built-in do shows but not consistent and with native indicator displayed at same time.
  2. 'When dragging a tab move it directly' has no effect for not checked state.
  3. While dragging things into the browser tab bar Firefox have the function of switch the current selected tab to the one mouse pointer hovering on after wait a while. This seems not working right with TMP DnD enabled, switch happened as soon as the mouse cursor passing the tab without any delay.
  4. No blank tab opened for .xpi file despite not using the option in TMP's link panel.

@onemen
Copy link
Owner

onemen commented Jan 27, 2021

I'm thinking of removing old tabmix style indicator
image

it not adding any better user experience

@117649
Copy link
Contributor Author

117649 commented Jan 27, 2021

I'm thinking of removing old tabmix style indicator
image

it not adding any better user experience

If so we may also be able to simplify tabbinding.js a little.

@onemen
Copy link
Owner

onemen commented Jan 27, 2021

Yes

@onemen
Copy link
Owner

onemen commented Jan 30, 2021

Maybe it is best to write the code from scratch.

We need to add support for:

  • multi-row
  • tabs with different width to calculate drop index
  • blocking link drop on locked tab (in dragover)
  • open link that will start a download in the current tab instead of new tab
  • block opening new window from detached tab in single window mode
  • hiding close button that is visible on hover only
  • ...

The current code was compatible with Firefox 55.

currently dragover dosn't work since aEvent.target.localName is never tab or tabs

      case "dragover": {
        let target = aEvent.target.localName;
        if (target != "tab" && target != "tabs")
          return;
        if (this.tabBar.useTabmixDnD(aEvent))
          TMP_tabDNDObserver.onDragOver(aEvent);
        break;
      }

@117649
Copy link
Contributor Author

117649 commented Jan 30, 2021

  • tabs with different width to calculate drop index

This one may not needed, current test shows that drop index is on the right position for single row with 'Tab width fit to tab tittle' enabled.

Now for some reason I don't know you may need to flip 'show close button on tab when mouse hover' switch off and on once after updated add-on for it to function.

@onemen
Copy link
Owner

onemen commented Jan 30, 2021

Now for some reason I don't know you may need to flip 'show close button on tab when mouse hover' switch off and on once after updated add-on for it to function.

this is the simplest of all, we just call TabmixTabbar.removeShowButtonAttr from dragstart

@117649
Copy link
Contributor Author

117649 commented Feb 1, 2021

Newest test on the Fx86 and latest commit shows 'built-in style Drop indicator' and 'When dragging a tab move it directly' are working.

@onemen
Copy link
Owner

onemen commented Feb 1, 2021

dragging tab from another window not working

@117649
Copy link
Contributor Author

117649 commented Feb 1, 2021

dragging tab from another window not working

I remember there seems a function that check dragging type may be take a look into that.

@onemen
Copy link
Owner

onemen commented Feb 1, 2021

my bad ....

-   if (aSourceNode.container == aSourceNode.ownerGlobal.gBrowser.tabContainer)
+   if (aSourceNode.container == gBrowser.tabContainer)

@117649
Copy link
Contributor Author

117649 commented Mar 2, 2021

// always check if the link is an xpi link
let filetype = ["xpi"];
if (TabmixSvc.prefBranch.getBoolPref("enablefiletype")) {
let types = TabmixSvc.prefBranch.getCharPref("filetype");
types = types.toLowerCase().split(" ")
.filter(t => filetype.indexOf(t) == -1);
filetype = [...filetype, ...types];
}

Why? I think when user turn it off then it should be completely off.

@onemen
Copy link
Owner

onemen commented Mar 2, 2021

we don't open new tab for left click on xpi links

@117649
Copy link
Contributor Author

117649 commented Mar 2, 2021

we don't open new tab for left click on xpi links

It also affect drag and drop which is inconvenient. Firefox are not very responsive regarding xpi dragged in from hard drive it just lost install panel randomly and many attempts may be required. Having a blank page with url helps a lot on retry install a xpi especially when you trying developing an addon .
And xpi is listed in the options it should obey that control it just shouldn't be hard coded.

@onemen
Copy link
Owner

onemen commented Mar 3, 2021

i will test when i finished updating drag and drop functions

it is most likely work if you drag a file onto about:addons

@117649
Copy link
Contributor Author

117649 commented Mar 7, 2021

Little change like this that close the condition statement works for me well.

           .filter(t => filetype.indexOf(t) == -1);
       filetype = [...filetype, ...types];
     }
+    else return false;
     var linkHrefExt = "";
     if (linkHref) {

@onemen
Copy link
Owner

onemen commented Aug 6, 2021

@117649 ,
Do you think we need this message, when dragging link to the on tabs ?

image

@117649
Copy link
Contributor Author

117649 commented Aug 6, 2021

@117649 ,
Do you think we need this message, when dragging link to the on tabs ?

image

Yes. If the function it stated are real and provide by TMP not Mozilla.

May be tweak the wording to shorter the sentence. It indeed pretty long for a tool tip.

@onemen
Copy link
Owner

onemen commented Aug 6, 2021

Yes. If the function it stated are real and provide by TMP not Mozilla.

When this function was introduced almost 14 years ago it was new feature, now days this is the standard action when dragging a link.

Tabmix block drop on locked tab, user can unlock the tab by pressing Ctrl/meta, but details about this feature and others should be in the documentation and not in tooltip

@onemen
Copy link
Owner

onemen commented Aug 7, 2021

I will change the code to show tooltip when link is dragged over a locked tab

  • for mac Hold ⌘ to allow link Url to replace locked tab
  • for others Hold Ctrl to allow link Url to replace locked tab

I hope you can suggest better wording...

@117649
Copy link
Contributor Author

117649 commented Aug 8, 2021

I will change the code to show tooltip when link is dragged over a locked tab

* for mac **Hold ⌘ to allow link Url to replace locked tab**

* for others  **Hold Ctrl to allow link Url to replace locked tab**

I hope you can suggest better wording...

eh. Hold Ctrl to replace locked tab with link Url ?

I'm not sure if this is grammar correct but at least a word shorter.

@onemen
Copy link
Owner

onemen commented Aug 8, 2021

🥇

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

No branches or pull requests

2 participants