Some fixes #246
Some fixes #246
Conversation
- show cached images right away
| [self cacheImage:image forPackage:package]; | ||
| if (completion) | ||
| completion(package, image); | ||
| }]; |
There was a problem hiding this comment.
Please use braces around these if statements
There was a problem hiding this comment.
I always use them, but that piece of code was just copy-pasted from project, I thought that it's coding style which you use.
There was a problem hiding this comment.
No, I think we agreed to always use them some time ago
|
Thanks @dev4dev! I'd like @kattrali or @supermarin to look this over. I saw a few things that I'm unsure about and commented inline, one other thing is we're now mixing tabs and spaces (it used to be all spaces). I don't really mind either way but it'd be nice if all code was indented the same. |
| dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(0.1 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ | ||
| // wait next run loop, otherwise there are no rows yet | ||
| [self loadImagesInVisibleRows]; | ||
| }); |
There was a problem hiding this comment.
Isn't this an indication that something somewhere else is going wrong?
There was a problem hiding this comment.
The cells will be visible only in the next runloop, so this is a solution I know. If you can recommend something better - I'd appreciate that :)
There was a problem hiding this comment.
Why load the table and then fetch images instead of loading the images when the cell is loaded? There's a bit more delay before all the images have been cached.
There was a problem hiding this comment.
Because I don't load images when cell loaded, only after slight delay when scroll ends. So if I scroll quickly through list, I wont get all them in loading queue.
Actually this part of code loads images in first visible cells after changing filtering predicate, because scroll event wont be fired at this point.
|
Fixes all the bugs, thanks! ✅ Filtering doesn't mix screenshots Though we have a slight issue now with scaling. Before the intended scale was to fit the image view horizontally, now all the images are tiny instead:
|
| } else { | ||
| [view.previewButton setImage:nil]; | ||
| } | ||
| view.screenshotPath = package.screenshotPath; |
There was a problem hiding this comment.
Definitely should keep indentation consistent with the project; soft tabs, 4 spaces per tab.
|
And would you be so kind to fix tabs -> space issue by yourself. I'm tab person and it's against my religion 8) |
|
There is no reason for anyone to change religions, but making your code conform to the existing codebase does not seem to be an unreasonable request. |
|
@dev4dev are you serious? |
|
@mokagio actually not 😸, but I do really don't like to use spaces for indentation 😈 |
|
lol |
|
Soooo ❓ |
|
removed |
|
Thanks for your PR. We'll try to get these fixes in soon :) |
I made some changes to screenshots preview, and they should fix these issues: #227 #230 #239