Presets

The "sort images" page is used to sort images overriding the 'default' order. I think it would be a good usability feature to have a few "presets" to choose from on that page.
Obvious choices would be
- sort by title
- restore the currently saved order
- reset the order (set weight=0 for all images in the gallery)

In my opinion, especially the last one is very important, as there is currently no way that I know of, to get back the original sorting once you have saved a custom sort.

Implementation

The right way to implement this in my opinion would be AJAX. The presets will need information that is not currently accessible in the DOM tree, and implementing this in the backend will allow us to implement arbitrary presets. The javascript layer would then order the images according to the data retrieved from the server, not yet saving the new order! This would be inline with the normal behavior of the sort images page. The difficult part will be the fallback, if we want that. Without jquery_ui, I will need to figure out a way to reorder the table, which should be possible, but will require a bit fiddling to set the weights accordingly. Without javascript, it would need to be a two-step form, like the node preview.

What I want to discuss before starting is:
- is this feature a good idea at all
- which presets would you deem important
- how far should our fall-back mechanism go, or should this be only for people having jquery_ui
- which version should we target

Cheers,
scroogie

CommentFileSizeAuthor
#20 ng3-get-images-quickfix.patch731 bytesscroogie
#13 ng3-sort-images-presets.patch16.95 KBscroogie
#10 ng3-sort-images-presets.patch16.81 KBscroogie
#6 ng3-sort-images-presets.patch12.64 KBscroogie

Comments

justintime’s picture

justintime commented

The 'Remove custom sorting' feature is a great idea, and I think it should go in ASAP. Even better, it won't take much code at all - just add a button using FAPI and a custom submit handler. By the way, I found the coolest function in core the other day: node_mass_update(). We've got a lot of code that mimics that behavior, what's your thoughts on either using that function or at least copying it's functionality?

With regards to the rest of the features, it's up to you. Personally, while it sounds cool when I first read it, I wonder what the practicality is of this. I'm not saying it's not useful, but I'm not seeing it. The problem I have is that it's a one time sort, and any adding of an image to the gallery will require re-visiting and re-applying the sort.

If implemented, it definitely belongs in NG core, and I would see no problem requiring jQuery UI for it. If you have a use case, or just want to work with it, by all means go ahead. I'll accept the patch as long as it works. However, if you don't have a use case and have anything else you'd like to work on, we can just mark this issue as postponed and see if any users request it.

Let's create a separate issue for the "remove custom sorting" feature, since I think we can isolate that as a nice, simple and quick feature.

Thoughts?

scroogie’s picture

scroogie commented

> The 'Remove custom sorting' feature is a great idea, and I think it should go in ASAP. Even better, it won't
> take much code at all - just add a button using FAPI and a custom submit handler.

Okay, but then we also need a cancel button I think. Otherwise, if we only have "Save" and "Reset" buttons, I'm sure people will confuse it with "Save" and "Cancel" (leaving the currently saved weights).

> By the way, I found the coolest function in core the other day: node_mass_update(). We've got a lot of code
> that mimics that behavior, what's your thoughts on either using that function or at least copying it's
> functionality?

Oh, I didn't know that either. I think that remedies an own issue. This can replace our node_gallery_batch_node_save().

> The problem I have is that it's a one time sort, and any adding of an image to the gallery will require
> re-visiting and re-applying the sort.

While that is true, I don't see why it should block this feature. If you leave the default sorting and add images, they're also just put on the end, they're not automagically sorted in.
What we could do is change the default weight, so that new images are always put to the end, even if custom weights are saved in. As the sort images page always starts at weight 0, we could set the default weight to 2147483647 (Max int). That would put new images to the end reliably.
I also think that people most of the time create new galleries, they don't add to existing galleries. In todays world, galleries are like news entries. You want them to be complete the first time the user visits them, because they'll most probably not visit it again. This is also one of the reasons I want hierarchies, so that you can organise a high amount of galleries.

> Personally, while it sounds cool when I first read it, I wonder what the practicality is of this. I'm not saying it's
> not useful, but I'm not seeing it.

Okay, I'll admit that it's of personal interest to me and my pet drupal project. :) But I think a lot of people might use their galleries in a similar fashion, so I'll try to explain:

For the page I mean a good friend is the content editor. He often complains to me how complicated his workflow is to create a gallery. The page has currently around 60.000 images in 370 galleries. A gallery usually represents an event (besides the historic archive, I'll explain that later). After an event, he looks at the photos in his usual photo management software, let's say Picasa. He selects the ones he wants to upload, and exports them. Now the first problem: If he sorted the images in Picasa, the sorting is lost on export. The file explorer and browser won't recognize that. Also, a lot of times the pictures come from different sources. Let's say three persons made photos of that evening. The pictures are named CIMGxyz or after some other crude naming scheme which doesn't help. Also, what happens most of the time, is that the EXIF date is completely useless as well, because the cameras are almost never in sync. If one is only 10 minutes off, the sort is broken.
So what he did is find another software, that is able to transfer a custom sorting to the filename. He loads all selected photos in that new software, orders them again, and renames the photos to something like eventxyz-0001 to eventxyz-0120. This step is really inconvenient. If this is now lost on upload again, because plupload somehow resorts the images, or the browser somehow sorts differently, he is going to murder me when there is no easy way to sort by title/filename. We have a lot of old image galleries, though, where this naming scheme is not yet the case. So we have to sort them differently. Also, we have the historic photos, which are all scanned in. Now and then, we get new photos that we put somewhere in the archive, mostly by year. As the pictures go back to the '30s, we have no idea which one comes first within a certain year. They get sorted by impression, just by what might fit where.

So what I need is: optional manual order per gallery and a quick way to order by title/filename just for one gallery which can also act as the basis for a manual order (so it shouldn't be a terminal state).

justintime’s picture

justintime commented

Well, users that submit patches are the projects highest priority, so if you code it, I'll accept it :) If you have an itch to scratch, someone else likely has it too.

One thing to clarify:

If you leave the default sorting and add images, they're also just put on the end, they're not automagically sorted in.

This isn't necessarily true. If the sort on the "image sort" display on the node_gallery_gallery_image_views is specified to sort by, say, title, then adding the new images will clear the cache, and upon next view it will be sorted by title. Right, or am I missing something?

scroogie’s picture

scroogie commented

Ah, indeed thats true. I just thought about the default sort order. I hope my other points are okay, though?

justintime’s picture

justintime commented

Yep, hack away :)

scroogie’s picture

scroogie commented
StatusFileSize
new ng3-sort-images-presets.patch12.64 KB

So here is a candidate. I could have implement this using Drupal AHAH forms as well, which would make it work with the tablesort, but the UI would not fit to the table, and it would complicate the form building etc. For me it's fine, it's a comfort feature for js users.
The only thing I don't like about this is the code duplication compared to the form function, though. Perhaps we can put that in a seperate function?

scroogie’s picture

scroogie commented
Status: Active » Needs review

Needs review.

justintime’s picture

justintime commented
Status: Needs review » Needs work

So, I didn't actually test this yet, because I ended up committing a bunch of stuff just after you created your patch, and it made a mess of the merge. However:

1) I committed your removal of node_gallery_get_all_image_nids().
2) I committed your renaming of node_gallery_cache_image_lists().

With regards to the code, I have a couple of namespace thoughts.

1) I hate it when things are called AJAX and there's no XML :).
2) For my jcarousel implementation, I'm going to need some json as well.

So, could we create the menu namespace 'node_gallery/json/*' to place all our json callbacks under. For your URL, you could use 'node_gallery/json/sort_images/%', and I could use 'node_gallery/json/get_sorted_nids/%'? Then your function can be named node_gallery_json_sort_images(), and mine can be node_gallery_json_get_sorted_nids(). Just for a little consistency. I'm open to suggestions on the 'sort_images' and 'get_sorted_nids' portions if you have other ideas.

If you can re-roll against HEAD, I promise not to commit anything until I hear back from you.

scroogie’s picture

scroogie commented

I'll upload a new patch incorporating your suggestions tomorrow.

scroogie’s picture

scroogie commented
StatusFileSize
new ng3-sort-images-presets.patch16.81 KB

So here is another try.

Points changed:
- modified menu path as discussed
- renamed functions as discussed
- moved redundant code to functions (you can probably use one of the functions directly for jcarousel)
- split logic and page function
- added a dynamic warning like tabledrag (using the same string for translations)

Surprisingly, the warning added nearly as much complexity as the rest of the feature, but it's a good thing to be consistent I guess. I also slightly modified the help text, first to remove the now redundant advice for saving, and second to clarify that this only modifies weights (if the user removed weight from the view, it will do nothing at all).

scroogie’s picture

scroogie commented
Status: Needs work » Needs review

Yeah, the status...

justintime’s picture

justintime commented
Status: Needs review » Needs work

Few things.

Your theme strings need wrapped in t().

Clicking the sort links don't do anything, and I get these warnings (in Firefox and Chrome, didn't test IE):

Warning: reference to undefined property b[E]
Source File: http://ng3.dev.techadvise.com/misc/jquery.js?S
Line: 13
Warning: reference to undefined property r[i]
Source File: http://ng3.dev.techadvise.com/misc/jquery.js?S
Line: 13
Warning: reference to undefined property window.ActiveXObject
Source File: http://ng3.dev.techadvise.com/misc/jquery.js?S
Line: 13
Warning: reference to undefined property window.ActiveXObject
Source File: http://ng3.dev.techadvise.com/misc/jquery.js?S
Line: 13
Warning: reference to undefined property b[E]
Source File: http://ng3.dev.techadvise.com/misc/jquery.js?S
Line: 13
Warning: reference to undefined property b[E]
Source File: http://ng3.dev.techadvise.com/misc/jquery.js?S
Line: 13

Last, let's do the ORDER BY in code for performance reasons, like we discussed.

scroogie’s picture

scroogie commented
StatusFileSize
new ng3-sort-images-presets.patch16.95 KB

Temporary new patch with t() and sorting in PHP instead of SQL before I jump into the javascript issue. I don't see a relation to my code from your errors, so I'll need to debug a bit.

scroogie’s picture

scroogie commented

Just for the protocol, I tried this in Chrome 7.0.517.44, Firefox 3.6.12 and Internet Explorer 8.0.7600.16385.

None of them gives me any errors. I'll search further.

scroogie’s picture

scroogie commented
Status: Needs work » Needs review

Page compression and Javascript optimization make no difference either, firebug and web developer see nothing to complain about. Sorry, I'll need some more hints until I can debug this.

justintime’s picture

justintime commented

I have family up for the next few days, so I won't be available for awhile. The only thing that I can think of was that I was on Linux when testing. I'll try to get some time to test from OS X and Windows on Sunday.

scroogie’s picture

scroogie commented

Okay, so I tried it out on my laptop, running OpenSUSE. No errors on Firefox 3.6.10, Konqueror 4.5.2, rekonq 0.6.0 and Chromium 6.0.443.0.

It must be something else. Perhaps it's a namespace issue again. Which other js files are loaded for you on that page?

justintime’s picture

justintime commented

Bah, I was in a hurry the other day, and didn't clear my caches, so the menu router cache wasn't rebuilt. Once I cleared the caches, it all worked fine. Reviewed, and committed to dev.

justintime’s picture

justintime commented
Status: Needs review » Fixed

marking fixed.

scroogie’s picture

scroogie commented
Status: Fixed » Needs review
StatusFileSize
new ng3-get-images-quickfix.patch731 bytes

Sorry, my mistake again. Small followup.

justintime’s picture

justintime commented
Status: Needs review » Fixed

Committed.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.