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

how to properly combine selector and sortFunction options #144

Closed
abubelinha opened this issue Jul 4, 2018 · 9 comments
Closed

how to properly combine selector and sortFunction options #144

abubelinha opened this issue Jul 4, 2018 · 9 comments

Comments

@abubelinha
Copy link

abubelinha commented Jul 4, 2018

This comes from #132 , but I think it might deserve a new issue title, since the sortFunction mentioned there does already work.
My problem comes only when I try to combine the selector and sortFunction options altogether.

The jsfiddles below reproduces the aforementioned "sort by text length" function.
For some reason, the "sort by length" button is failing when I am trying to use it to sort a list using a span as selector. Has the following tinysort() call an incorrect syntax?

<div id=mydiv>
   <ol>
      <li><span>1</span> <span class=myclass>abcde</span></li>
      <li><span>2</span> <span class=myclass>bcde</span></li>
      <li><span>3</span> <span class=myclass>zyx</span></li>
      <li><span>4</span> <span class=myclass>z</span></li>
   </ol>
</div>

<script type=text/javascript>
function sortByLength(a,b) {
   var lena = a.elm.textContent.length
   var lenb = b.elm.textContent.length;
   return lena === lenb ? 0 : ( lena>lenb ? 1 : -1);
}
tinysort("div#mydiv>ol>li", {selector:'span.myclass', sortFunction: sortByLength});
</script>

I would expect the list to be sorted upside-down after the tinysort() call, based on the string lengths inside the right spans ... but when I try to call this from the "sort by length" button in my jsfiddle, it does not work:

Please note the jsfiddle also needs charorder plugin to be used in another button. So I tried to test differences in loading or not loading that plugin, although it shouldn't change the behaviour of the "sort by length button" (since the tinysort call in it does not use charorder):

@Sjeiti
Copy link
Owner

Sjeiti commented Jul 5, 2018

Verified bug http://jsfiddle.net/Sjeiti/cfoqtk7z/
duplicate of #122

@Sjeiti Sjeiti closed this as completed Jul 5, 2018
@abubelinha
Copy link
Author

abubelinha commented Jul 10, 2018

Thanks @Sjeiti , but that bug does not explain why the 2nd jsffidle above fails: it does NOT use charOrder script.

I made a much simpler version showing just the button which fails running sortFunction option:
https://jsfiddle.net/abubelinha/ovr6cn21/9

What is failing there?

@Sjeiti
Copy link
Owner

Sjeiti commented Jul 12, 2018

You expect the custom sort function to sort a subselection when in fact it sorts the length of the entire list-element. The custom sort function ignores all other properties you set because it expects you to implement these yourself. This is by design.
But I'll update the documentation to note that all other properties will be ignored.

@abubelinha
Copy link
Author

abubelinha commented Jul 13, 2018

tinysort("div#mydiv>ol>li", {sortFunction: sortByLength, selector:'span.myclass'});`

So the "selector" parameter above has no effect? sortByLength(a,b) should take care of finding the span.myclass element inside the li elements received as parameters (a,b), and measuring its content length or whatever I want to do to sort the list. Correct?

Thanks a lot for your explanation @Sjeiti. Now it works: https://jsfiddle.net/abubelinha/ovr6cn21/35/

Also, there was a 2nd question I raised in the original message. I think you didn't answer yet:

Do you know why sortFunction does not work if the charOrder plugin script is loaded? (even if I don't run its code).
This was the provided jsfiddle: https://jsfiddle.net/abubelinha/kcfpu3sr/161/

@Sjeiti
Copy link
Owner

Sjeiti commented Jul 14, 2018

Still working on that last one, I did fix one charorder issue but there is still one thing wrong

@abubelinha
Copy link
Author

Thanks @Sjeiti
Does that bug have an open issue link which I can follow?
I am not sure where I should look at: this issue and the other you linked to are already closed.

@Sjeiti
Copy link
Owner

Sjeiti commented Jul 16, 2018

@abubelinha I don't have time to look into it this week but when I do I'll refer you when it's fixed.

As for your question regarding passing params: there is one open pull request touching that issue. I don't think I'll merge it as is, but I might extend the sortFunction with similar functionality... maybe...

@abubelinha
Copy link
Author

abubelinha commented Aug 14, 2020

@abubelinha I don't have time to look into it this week but when I do I'll refer you when it's fixed.

Any advances? I recall the status of my question: custom sort functions failed when charOrder plugin was loaded.
As I see a later (but also old) #153 issue related to charOrder plugin, now I wonder if that is related to my issue.

Or perhaps mine was simply a collateral effect of what you said above about sortFunction? (so any sortFunction will ignore charOrder "by design", and not due to a fail in any of them):

The custom sort function ignores all other properties you set because it expects you to implement these yourself. This is by design.


And what about this other pending question? You mentioned a related pull request (I don't know the link):

As for your question regarding passing params: there is one open pull request touching that issue. I don't think I'll merge it as is, but I might extend the sortFunction with similar functionality... maybe...

I am specially interested in this, because a custom sortFunction can also be used to somehow resemble the failing charOrder plugin (at least in the mean time, if the issues above still persist).

For example, I have now a custom myAZcharOrderFunction() which works perfectly with TinySort for sorting elements based on their inner text (a.elm.textContent vs b.elm.textContent).
But as its code sorts by .elm.textContent I can't reuse the same function for sorting elements by their title attribute, for example (I would need to create a different function):

It works great like this:
tinysort('div.mynames>span.name', {sortFunction: myAZcharorderFunction});

But this wouldn't work:
tinysort('div.mynames>span.name', {attr:'title', sortFunction: myAZcharOrderFunction});
tinysort('div.mynames>span.name', {attr:'title'}, {sortFunction: myAZcharOrderFunction});

BTW ... which one of the last two calls is the correct syntax? I guess the 1st one, but I still have not a clear understanding of when using separate curly brackets. This example confuses me a bit:

tinysort('ul#xmul>li','span.name',{selector:'span.date',data:'timestamp'});

... would any of these make any different effect?

tinysort('ul#xmul>li',{selector:'span.name'},{selector:'span.date'},{data:'timestamp'});
tinysort('ul#xmul>li',{selector:'span.name',selector:'span.date'},{data:'timestamp'});
tinysort('ul#xmul>li',{selector:'span.name',selector:'span.date',data:'timestamp'});

@abubelinha
Copy link
Author

As for your question regarding passing params: there is one open pull request touching that issue. I don't think I'll merge it as is, but I might extend the sortFunction with similar functionality

@Sjeiti have you any solution for passing parameters (i.e. a selector) to sortFunction?

My example in https://jsfiddle.net/abubelinha/ovr6cn21/35/ can sort elements based on a given selector, but its name has to be hardcoded inside the custom function and that is not a general solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants