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

Add list links horizontal draggable procedure #8397

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

pmario
Copy link
Member

@pmario pmario commented Jul 17, 2024

This PR adds a new list-links-horizontal-draggable procedure, which can be used to:

For larger screens it looks like this:

image

For mobile screens:

  • The last position drop area is now below the list, so it should be easy to move an item to the end of the list
  • The standard drop positions are much larger then for PC screens. So the can be seen below fingers.
    • At least it works for me

image

Copy link

vercel bot commented Jul 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
tiddlywiki5 ✅ Ready (Inspect) Visit Preview Jul 17, 2024 0:50am

Copy link

Confirmed: pmario has already signed the Contributor License Agreement (see contributing.md)

@Jermolene
Copy link
Member

Thanks @pmario. It is a shame that there is so much duplication with the existing list-links-draggable macro; I would have hoped that a slight modification/extension of that existing macro would be able to handle horizontal lists.

A small point, but it looks like the drop area labelled "last position drop area" is "display: block". It would be nice if the entire control could be "display: inline".

@pmario
Copy link
Member Author

pmario commented Jul 17, 2024

A small point, but it looks like the drop area labelled "last position drop area" is "display: block". It would be nice if the entire control could be "display: inline".

That's the "last position drop area" that is only visible for small screens, as seen in the screenshot. I was unable to use the "last drop position" in horizontal mode with my mobile phone. So I moved it to the bottom. Now its easy to move any element to the end of the list.

@Jermolene
Copy link
Member

That's the "last position drop area" that is only visible for small screens, as seen in the screenshot. I was unable to use the "last drop position" in horizontal mode with my mobile phone. So I moved it to the bottom. Now its easy to move any element to the end of the list.

I don't think that media queries are the right solution here. Media queries can only respond to the size of the entire window. That's OK for components like tiddlers that are intended to be displayed at or near the full width of the window. But it doesn't work well for generic, reusable components. For example, if the list-links-horizontal-draggable procedure were to be used in a multicolumn layout on a tablet then users might want the smartphone layout.

On a separate note, I should have mentioned before but we should call this list-links-draggable-horizontal so that it is next to the existing list-links-draggable macro in alphabetic order.

@kookma
Copy link
Contributor

kookma commented Jul 18, 2024

Can't we use CSS to make list-links and list-tags horizontal?
See https://talk.tiddlywiki.org/t/how-to-create-a-horizontal-list-links-draggable/10204

I think the current code is enough complex, I would suggest making the code simpler and use CSS as much as possible to make list horizontal

@pmario
Copy link
Member Author

pmario commented Jul 18, 2024

I think the current code is enough complex, I would suggest making the code simpler and use CSS as much as possible to make list horizontal

I did make the list-links-horizontal-draggable procedure for 1 reason: "To make it possible to drag-sort the tag-list in the EditTemplate". The rest is a "by-product".

Your idea with display:flex is brilliant and I can probably use it. But it can not move an item to the end of the list. So it's not finished. As always. The devil is in the details.

The bigger picture

To make list-links-draggable work in the tag-editor, we need to significantly modify the list-links-draggable macro. If we do that, we need to make it work with TW v5.3.x syntax. $:/core/macros/list tiddler contains 5 macros at the moment:

  • list-links
  • list-links-draggable
  • list-links-tagged-draggable
  • list-tagged-draggable-drop-actions
  • list-linked-draggable-drop-actions

Those 5 macros are widely used and probably used / extended by plugin authors. So backwards compatibility will go ballistic. Especially since I think "...tagged-draggable" should be a wrapper for "list-links-draggable". But that's a completely unrelated project on its own.

I would probably need 20% of the time to make it work and then spend 80% of the time to make it compatible again. From experience I know touching existing UI code causes pain.

As I wrote, I did create the list-links-horizontal-draggable procedure for 1 reason. To make it work in the EditTemplate tag-editor. So going with a completely new procedure that supports an itemTemplate= variable-name or tiddler-title is the easiest way to start with.

The next step is: $:/core/ui/EditTemplate/tags

$:/core/ui/EditTemplate/tags is a template and transcluded into the EditTemplate. Since it's a template and it could only be used with transclusions, rewriting it did not cause any known compatibility problems.

I did rewrite the tags.tid 5 months ago for a related reason: To make its UI (see screenshot below) usable (in the future) with any kind of list-like fields instead of the default: tag field only.

image

To make that UI more flexible we need to make the tag-picker macro more flexible and easier to change. So I did rewrite it 4 months ago -- and then needed to fix a bug for a usecase I did not test. This was one of the reasons why we needed a v5.3.5 bugfix release. -- Luckily it was not the only reason.

So -- It's not that easy.

Copy link
Member

@Jermolene Jermolene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work on this @pmario

<<_dragHandle>>
</span>
<% endif %>
<% if [<itemTemplate>is[variable]] %>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not at all keen on overloading the itemTemplate parameter in this way. No existing core macro/procedure works like this, and I think it is guaranteed to lead to unexpected and hard to debug errors. It would be clearer to have two separate parameters, with the precedence between them documented.

</$let>
\end

\procedure list-links-horizontal-draggable(tiddler,field:"list",emptyMessage,type:"span",subtype:"span",class:"",itemTemplate,dragHandle:"⠿",enable:"yes")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not appropriate to use Unicode characters as UI elements. Unicode characters have semantic meaning, and here those semantics are inappropriate.

I think we need a new $:/core/images/drag-handle SVG image.

@pmario pmario marked this pull request as draft July 18, 2024 15:45
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

Successfully merging this pull request may close these issues.

3 participants