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

[IDEA] Add mobile support for draggable tiddlers #5102

Open
cipharius opened this issue Nov 23, 2020 · 24 comments
Open

[IDEA] Add mobile support for draggable tiddlers #5102

cipharius opened this issue Nov 23, 2020 · 24 comments

Comments

@cipharius
Copy link

Is your feature request related to a problem? Please describe.
Draggable lists work well on desktop browsers, but are not supported on mobile web browsers.

Describe the solution you'd like
Additionally to dragstart and dragend events, listen for touchstart, touchmove and touchend. $:/core/modules/utils/dom/dragndrop.js can be refactored so that same logic can be reused for both touch based and drag based events.

Describe alternatives you've considered
N/A

Additional context
N/A

@Jermolene
Copy link
Owner

Hi @cipharius the situation with drag and drop is a bit patchy across different platforms; for example, it does work on iPad since (I think) iOS 13, but it doesn't work on iPhones. What platforms are you interested in?

@cipharius
Copy link
Author

I'm specifically interested in Android chrome browser support, so that I could use TiddlyWiki as PWA.

@Jermolene
Copy link
Owner

Hi @cipharius this table suggests that recent Chrome for Android does support drag and drop:

https://caniuse.com/dragndrop

What version are you using?

@cipharius
Copy link
Author

That's odd, attempting to drag items in draggable todo list example just scrolls the page, or when held too long, shows context menu. Searching online does not mention any caveats to take care of.

I just tested drag and drop example from MDN and it works. I think the issue here is that instead of initiating the drag when holding down on an item, it instead opens context menu for the link.

The MDN drag event example: https://developer.mozilla.org/en-US/docs/Web/API/Document/dragend_event
Android chrome version: 86.0.4240.198

@saqimtiaz
Copy link
Contributor

I can confirm that the MDN example works for me but drag and drop in TW does not. Chrome for Android 86

@saqimtiaz
Copy link
Contributor

I suspect the difference might be explained by the fact that most draggable items in TW are links whereas the MDN example uses DIVs.

@cipharius
Copy link
Author

I have found a workaround for this problem, perhaps it can give an idea how to properly handle draggable tiddler lists.

My setup is similar to the example draggable ToDo list except each item is wrapped in a div with attributes <div draggable="true" class="custom-draggable">.

Then have this script snippet with $:/tags/RawMarkup tag:

<script>
  document.addEventListener('dragstart', e => {
    if (e.target && e.target.classList.contains("custom-draggable")) {
      let tiddler = e.target.querySelector(".tc-tiddlylink");
      e.dataTransfer.setData('text/plain', tiddler.innerText);
    }
  });
</script>

Now it's possible to drag list items on android chrome, by longpressing on the space next to tiddler title.

@saqimtiaz
Copy link
Contributor

@cipharius thank you for that. It helps confirm that the issue is indeed the contextmenu being triggered on links instead of drag.

Another workaround could be to wrap the links in a <$draggable> widget to create the div, removing the need for the extra JavaScript: https://tiddlywiki.com/#DraggableWidget

@cipharius
Copy link
Author

@saqimtiaz Thank you, I'm still finding my way around TiddlyWiki architecture.

Using DraggableWidget I got this task template:

<$draggable filter="[<currentTiddler>]">
<$checkbox tag="done"> <$link/> </$checkbox>
</$draggable>

While this works on desktop, for some reason on mobile it adds extra [[ and ]] around the tiddler title. Same with title={{!!title}}.

@saqimtiaz
Copy link
Contributor

@cipharius So the draggable widget payload is a title list rather than a tiddler title. In a title list any tiddler title with spaces is enclosed in double brackets.

So the key here would be handling the title list (and brackets) in the actions that occur when the drag ends. Are you using a droppable widget? If you post the entire code for your draggable list we can workout a solution. Alternatively you could post a TiddlyWiki file with the relevant code as well.

@saqimtiaz
Copy link
Contributor

@cipharius it also occurs to me that the <$link> widget supports a "tag" attribute so we could also try having the link rendered as a div or span instead as well, avoiding the need for the <$draggable> widget.

@cipharius
Copy link
Author

I tried changing tag of link widget and I still get the issue, where extra [[ and ]] are added. My whole setup looks like this:

TaskTemplate:

<$checkbox tag="done"> <$link tag="span"/> </$checkbox>

WikiText for the list:

<$macrocall $name="list-tagged-draggable" tag={{!!title}} subFilter="tag[task]!tag[done]!has[draft.of]" itemTemplate="TaskTemplate" emptyMessage="No tasks."/>

My intent is to group tasks by project and let the tasks have different order in each project.

@saqimtiaz
Copy link
Contributor

@cipharius apologies for the late reply. I needed to find the time to do some debugging.

You are correct. The actionTiddler variable in the droppable widget always contains an extra opening and closing double brackets in Chrome on Android even though it should be a simple tiddler title with no extra encoding. That is, for tiddler My title the actionTiddler is [[My title]] which when used within the list-links-draggable macro gets saved to the tags field as [[[[My title]]]] !!

Any thoughts on this @Jermolene ? I assume we always wrap tiddler titles with spaces, irrespective of whether they already are wrapped in brackets, to support titles like: [[title with brackets]] ?

So interestingly, on Android Chrome, the event.dataTransfer property for the drag event only contains one type which is text/plain and contains the tiddler title in title list format, that is if it contains spaces it is in double brackets. My title becomes [[My title]]. However the droppable widget expects the title not to have the double brackets.

On Desktop, the same event.dataTransfer property also contains the types text/uri, text/x-moz-url and most importantly text/vnd.tiddler which contains the tiddler in JSON format and the title field does not have any extra encoding. This is why on Desktop the actionTiddler variable does not contain extra brackets and the macro works as it should.

One kludgy workaround is to use the 5.1.23 pre-release version and customize the list-tagged-draggable-drop-actions macro in $:/core/macros/list as follows: (Note the first and last two lines of the macro).

\define list-tagged-draggable-drop-actions(tag)
<$vars startBrackets="[[" endBrackets="]]">
<$vars actionTiddler={{{[<actionTiddler>trim:suffix<endBrackets>trim:prefix<startBrackets>]}}}>
<!-- Save the current ordering of the tiddlers with this tag -->
<$set name="order" filter="[<__tag__>tagging[]]">
<!-- Remove any list-after or list-before fields from the tiddlers with this tag -->
<$list filter="[<__tag__>tagging[]]">
<$action-deletefield $field="list-before"/>
<$action-deletefield $field="list-after"/>
</$list>
<!-- Save the new order to the Tag Tiddler -->
<$action-listops $tiddler=<<__tag__>> $field="list" $filter="+[enlist<order>] +[insertbefore:currentTiddler<actionTiddler>]"/>
<!-- Make sure the newly added item has the right tag -->
<!-- Removing this line makes dragging tags within the dropdown work as intended -->
<!--<$action-listops $tiddler=<<actionTiddler>> $tags=<<__tag__>>/>-->
<!-- Using the following 5 lines as replacement makes dragging titles from outside into the dropdown apply the tag -->
<$list filter="[<actionTiddler>!contains:tags<__tag__>]">
<$fieldmangler tiddler=<<actionTiddler>>>
<$action-sendmessage $message="tm-add-tag" $param=<<__tag__>>/>
</$fieldmangler>
</$list>
</$set>
</$vars>
</$vars>
\end

With this change and no extra JavaScript needed you can use the following task template:

<$checkbox tag="done"></$checkbox> <$link tag="span" draggable="yes" to={{!!title}}/>

@Jermolene
Copy link
Owner

You are correct. The actionTiddler variable in the droppable widget always contains an extra opening and closing double brackets in Chrome on Android even though it should be a simple tiddler title with no extra encoding. That is, for tiddler My title the actionTiddler is [[My title]] which when used within the list-links-draggable macro gets saved to the tags field as [[[[My title]]]] !!

Any thoughts on this @Jermolene ? I assume we always wrap tiddler titles with spaces, irrespective of whether they already are wrapped in brackets, to support titles like: [[title with brackets]] ?

Very strange. makeDraggable() in $:/core/modules/utils/dom/dragndrop.js just calls $tw.utils.stringifyList() to add the double square brackets, as can be seen by dragging a tag pill into a text area:

image

It's true that $tw.utils.stringifyList() will wrap double square brackets around titles that already are wrapped in double square brackets, but I don't think that's the problem here.

Is it just that the droppable widget is expecting a title, while the draggable widget is providing a title list?

So interestingly, on Android Chrome, the event.dataTransfer property for the drag event only contains one type which is text/plain and contains the tiddler title in title list format, that is if it contains spaces it is in double brackets. My title becomes [[My title]]. However the droppable widget expects the title not to have the double brackets.

On Desktop, the same event.dataTransfer property also contains the types text/uri, text/x-moz-url and most importantly text/vnd.tiddler which contains the tiddler in JSON format and the title field does not have any extra encoding. This is why on Desktop the actionTiddler variable does not contain extra brackets and the macro works as it should.

That makes it sound as if the problem is some inconsistency with the importDataTypes defined in $:/core/modules/utils/dom/dragndrop.js. The system calls the first handler that matches a provided type, so presumably on mobile the text/plain parser is being used and on mobile while the text/vnd.tiddlywiki is being used on desktop.

@saqimtiaz
Copy link
Contributor

Is it just that the droppable widget is expecting a title, while the draggable widget is providing a title list?

@Jermolene that is the symptom rather than the root problem as it is only in Chrome on Android that the draggable widget provides a title list, on desktop it provides a title. I believe this is due to the different type used from the event.dataTransfer

That makes it sound as if the problem is some inconsistency with the importDataTypes defined in $:/core/modules/utils/dom/dragndrop.js. The system calls the first handler that matches a provided type, so presumably on mobile the text/plain parser is being used and on mobile while the text/vnd.tiddlywiki is being used on desktop.

So I've confirmed that utils.makeDraggable is setting up all the different types in event.dataTransfer including text/vnd.tiddlywiki. However, in droppable.handleDropEvent the event.dataTransfer property contains only the type text/plain.

So the only type available to import to importDataTypes is text/plain which has the title in title list format. Whereas on desktop we use the text/vnd.tiddlywiki type and therefore do not get the title in title list format.

I can try to do some more debugging later today. I welcome any thoughts or suggestions.

@saqimtiaz
Copy link
Contributor

@Jermolene I'm wondering if this might be a bug with Chrome on Android and how it handles event.dataTransfer.setData for different types. I'll investigate as time allows and report back.

@Jermolene
Copy link
Owner

Thanks @saqimtiaz this is a tricky area. I recall that the combination (and ordering) of the types that are set and retrieved was very fiddly when I first set things up.

One point to remember is that we need to be able to drag draggable things into text areas and get a title list (including textareas outside TiddlyWiki), so we can't do things like overload the text type with a JSON representation.

@saqimtiaz
Copy link
Contributor

One point to remember is that we need to be able to drag draggable things into text areas and get a title list (including textareas outside TiddlyWiki), so we can't do things like overload the text type with a JSON representation.

Agreed. If it is a browser bug and we do decide to ameliorate it, I would favour trying to make the wiki text macros more flexible regarding their input variables. Ultimately it may be that we should not do more than just document the idiosyncrasy if it is a browser inconsistency.

@saqimtiaz
Copy link
Contributor

saqimtiaz commented Dec 9, 2020

@Jermolene I can confirm that is a browser issue and not a TiddlyWiki bug.

I modified the MDN drag example to set two event.dataTransfer types, text/plain and text/vnd.tiddlywiki in dragStart. In the drop event only text/plain was available in Chrome on Android.

I do not think there is much we should do to try and work around this. My only thought is whether in makeDraggable() in $:/core/modules/utils/dom/dragndrop.js we might call $tw.utils.stringifyList() only when we have more than one title?

The impact would be minimal for non-mobile Chrome use cases but it would resolve this problem and make drag usable on Android.

@Jermolene
Copy link
Owner

Thanks @saqimtiaz

I modified the MDN drag example to set two event.dataTransfer types, text/plain and text/vnd.tiddlywiki in dragStart. In the drop event only text/plain was available in Chrome on Android.

Could it be that Chrome on Android has an accept list of content types that it will accept? I recall that's why we put some of the setData calls behind an IE conditional.

I do not think there is much we should do to try and work around this. My only thought is whether in makeDraggable() in $:/core/modules/utils/dom/dragndrop.js we might call $tw.utils.stringifyList() only when we have more than one title?

That sounds like it might break things elsewhere? If we don't call stringifyList then the other end can't distinguish between dragging "A B C" and "[[A B C]]"?

The impact would be minimal for non-mobile Chrome use cases but it would resolve this problem and make drag usable on Android.

It would certainly be great to get it working.

@saqimtiaz
Copy link
Contributor

saqimtiaz commented Dec 9, 2020

Could it be that Chrome on Android has an accept list of content types that it will accept? I recall that's why we put some of the setData calls behind an IE conditional.

Not that they have documented anywhere. However there are many bug reports going years back of the same problem in Chrome for Desktop, where only the type text/plain and text was supported in event.dataTransfer, before eventually being resolved.
I have not found any discussion regarding the mobile implementation but it seems likely the same issue is still present in mobile Chrome. I'll raise an issue on the Chromium repo when I get the chance.

I do not think there is much we should do to try and work around this. My only thought is whether in makeDraggable() in $:/core/modules/utils/dom/dragndrop.js we might call $tw.utils.stringifyList() only when we have more than one title?

That sounds like it might break things elsewhere? If we don't call stringifyList then the other end can't distinguish between dragging "A B C" and "[[A B C]]"?

Good point. I'll do some testing post 5.1.23 and see if there is a way we can mitigate this problem.

It is good to know though that the problem isn't in our handling of the event.

@Jermolene
Copy link
Owner

Thanks @saqimtiaz much appreciated (I've always found working on drag and drop rather dispiriting because of the rampant browser incompatibility).

@saqimtiaz
Copy link
Contributor

saqimtiaz commented Dec 9, 2020

PS: I was thinking mostly in terms of the DroppableWidget which always expects the actionTiddler variable to be a single title when I suggested skipping the call to $tw.utils.stringifyList(). So it would work just fine there, but I see that this could be problematic elsewhere like dragging a link into a text-editor.

I empathize with you on the frustrations of browser incompatibility in this area, which is why I want to see if there is a relatively easy workaround, but beyond that I do not think we should extend ourselves over this.

@pmario
Copy link
Contributor

pmario commented Sep 9, 2022

This problem is related to that PR "review but don't merge: fix dropzone closure variable problem" #6622

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

4 participants