Only show sort buttons if there is more than 1 image in the album.

[virajparimi]

Fixes: #573

Licence: AGPL

Description

Fixes the problem and shows sort button if more than 1 image in album.

Features

Works for images as well as folders.

Tested on

• Ubuntu 14.04/Chrome

Reviewers

@oparoz

[mention-bot]

By analyzing the blame information on this pull request, we identified @oparoz to be a potential reviewer

[oparoz]

Fixes the problem and shows sort button if more than 1 image in album.

It would be good if you could describe in more details the logic you've used. That way the principles can be validated before the code.

Looking at the code, I see a couple of issues:

1. The first conditions shows the buttons if there are 0 albums or images
2. The 2nd conditions allows for 2 elements to be shown on screen

So it seems you need to write more tests in your test plan to try and cover more cases

[virajparimi]

@oparoz First condition is pretty trivial to fix. But the second condition is for when 1 folder and 1 image is present. At that time should we show sort buttons or not?

[oparoz]

when 1 folder and 1 image is present. At that time should we show sort buttons or not?

Yes. Even though it appears that no sorting would be necessary, because all elements can bee seen at once, I have no way of defining from which point it is.
Should people not have the capability to sort less than 5 elements? 3? 10? I don't think we can arbitrarily pick one.

[oparoz]

Ah and I read the 2nd condition wrong. So, the logic is correct there, but try to combine 1 & 2 if you can.

If we have more than one element -> show buttons

[virajparimi]

@oparoz Have a look at this one.

[oparoz]

That doesn't work if I have 10 images and 0 albums, no? (Add this case to the test plan if it doesn't)

[virajparimi]

@oparoz My bad, I missed those conditions. Anyways the new one adds those conditions as well.

[oparoz]

OK, now try to combine these conditions into 1, if possible.

[virajparimi]

@oparoz Have done. Please check. Thanks!

[oparoz]

I think I have a better idea. Add the number of images and albums and if the sum is greater than one, then show the buttons. What do you think?

[virajparimi]

@oparoz 👍 Certainly a good idea, as it decreases the number of redundant comparisions and combines them. Have updated the file.

[oparoz]

OK about the idea, but have you tested your code? By looking at it, I'd say the way you wrote it doesn't work.

[virajparimi]

@oparoz Sorry for the inconvenience.

[oparoz]

We all make mistakes :), but that's why I think the test plan is important. It makes it easy to quickly review all cases.

Let me know when you've tested the solution and I'll review it.

[virajparimi]

@oparoz I have tested it and as far as I can see there are no errors. I worked with 1 image, then 2 images, similarly for albums and then both of the together.

[oparoz]

OK, I'll let you summon some testers ;)

[virajparimi]

Ok, @imjalpreet @tahaalibra Need your reviews about this. What do you think?

[tahaalibra]

i think @mbtamuli was working on this. please review...
i will test this and comeback to you

[mbtamuli]

Everything is working properly. 👍

• 1 Image.
• 1 Folder.
• 1 Image + 1 Folder
• Multiple Images
• Multiple Folders
• Multiple Images + Multiple Folders

[oparoz]

Thanks @mbtamuli