Folder preview image will be taken from all child directories not only direct childs #165
Conversation
lexas2509
changed the title
|
Inspired by this issue [https://github.com//issues/154] this MR is fixing blank folder thumbnail while there are the non-empty subfolders (but the folder itself doesn't have any images) |
|
I looked at the code. I have to double check, but I think this approach leads to inconsistent UI. The inconsistency can come from showing c.jpg in DirB (not only for preview). |
|
After I gave some thought to it. You might not notice the inconsistency, but I still find this solution a bit misleading (namely you use a variable (.media) to store a image in it that is not part of a given directory. A more proper solution would be to add a new filed to the directory here: like previews: S[];And store the previews for a given directory there. Should not be too hard to implement, just a lot of refactoring. |
| @@ -30,7 +30,7 @@ export class ConfigDiagnostics { | |||
| if (databaseConfig.type !== ServerConfig.DatabaseType.memory) { | |||
| await SQLConnection.tryConnection(databaseConfig); | |||
| } | |||
| if (databaseConfig.type !== ServerConfig.DatabaseType.sqlite) { | |||
| if (databaseConfig.type === ServerConfig.DatabaseType.sqlite) { | |||
There was a problem hiding this comment.
This is a really nice catch. I would e happy to merge this change even separately from the other changes.
There was a problem hiding this comment.
I can prepare a separate change for this if you want.
There was a problem hiding this comment.
I've tested the actual fix several times and it works fine. No inconsistencies found. Sure, it would be better to have something like a dedicated previews attribute, however it seems to be a lot of work.
I would also suggest to move the fix of 'ConfigDiagnostics.ts' into another pull request and merge the actual one. @lexas2509 Can you do it in your pull request, otherwise I can do another one to get the fix soon merged :-)
There was a problem hiding this comment.
Alright. Fingers crossed. I'll merge this.
Worst case the nightly build wont work. There isn't much in it compared to 1.8.2.
I wanted to fix this actually properly. Now I will be forced to.
I might just remove the memory (non DB) approach. That will make things simpler and with docker, having at least an sqlite DB, should not be an issue. Although, I'm not sure how may people are using it.
There was a problem hiding this comment.
Thanks! Tested the result of the nightly build and it works quite well on my side.
No description provided.