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

Thumbnails utiliy => Get folders #2645

Closed
wouterverstuyf opened this Issue Oct 1, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@wouterverstuyf
Contributor

wouterverstuyf commented Oct 1, 2018

Type

Critical bug

Problem description

When using the new Thumbnails utility for getting the folders there are 2 problems:

  • the folders are listed multiple times
  • when you include the source folder, only the source folders gets returned, the other folders are not listed

Steps to reproduce

Execute the code:
BackendModel::get(Thumbnails::class)->getFolders(<IMAGE_PATH>, true);

Solution

I have changed the function, but probably there is more optimalisation possible. To fix the doubles the Finder utility needs to be reinitialized.
To add the source folder I have added a second condition.

Feel free to adjust and make it 100% correct.

`public function getFolders(string $inPath, bool $includeSourceFolder = false): array
{
if (!$this->filesystem->exists($inPath)) {
return [];
}

    $folders = [];
    $nameFilter = '/^([0-9]*)x([0-9]*)$/';

    $finder = new Finder();
    foreach ($finder->directories()->in($inPath)->name($nameFilter)->depth('== 0') as $directory) {
        $dirname = $directory->getBasename();
        $chunks = explode('x', $dirname, 2);

        if (count($chunks) !== 2) {
            continue;
        }

        $folders[] = [
            'dirname' => $dirname,
            'path' => $directory->getRealPath(),
            'width' => is_numeric($chunks[0]) ? (int) $chunks[0] : null,
            'height' => (array_key_exists(1, $chunks) && is_numeric($chunks[1])) ? (int) $chunks[1] : null,
            'url' => (0 === mb_strpos($inPath, $this->sitePath))
                ? mb_substr($inPath, mb_strlen($this->sitePath)) : ''
        ];
    }

    if ($includeSourceFolder) {
        if ($this->filesystem->exists($inPath . '/source')) {
            $folders[] = [
                'dirname' => 'source',
                'path' => $inPath . '/source',
                'width' => '',
                'height' => '',
                'url' => (0 === mb_strpos($inPath, $this->sitePath))
                    ? mb_substr($inPath, mb_strlen($this->sitePath)) : ''
            ];
        }
    }

    return $folders;
}

`

@StijnVrolijk

This comment has been minimized.

Show comment
Hide comment
@StijnVrolijk

StijnVrolijk Oct 3, 2018

Contributor

Actually, lately we've been using imagine filter functionality instead of the 'pregenerating' thumbnails. It all depends on what you need of course but I suggest looking into imagine filters, it's fully compatible with Fork.

https://symfony.com/doc/2.0/bundles/LiipImagineBundle/filters/sizing.html#thumbnails

<img src="{{ myImageSource|imagine_filter('my_custom_filter') }}" />

for example :)

Contributor

StijnVrolijk commented Oct 3, 2018

Actually, lately we've been using imagine filter functionality instead of the 'pregenerating' thumbnails. It all depends on what you need of course but I suggest looking into imagine filters, it's fully compatible with Fork.

https://symfony.com/doc/2.0/bundles/LiipImagineBundle/filters/sizing.html#thumbnails

<img src="{{ myImageSource|imagine_filter('my_custom_filter') }}" />

for example :)

@wouterverstuyf

This comment has been minimized.

Show comment
Hide comment
@wouterverstuyf

wouterverstuyf Oct 5, 2018

Contributor

@StijnVrolijk
Yes, I have seen the imagine filters in the medialibrary module for example.
So the "old" way of generating thumbnails will dissapear and is no longer supported?

Contributor

wouterverstuyf commented Oct 5, 2018

@StijnVrolijk
Yes, I have seen the imagine filters in the medialibrary module for example.
So the "old" way of generating thumbnails will dissapear and is no longer supported?

@StijnVrolijk

This comment has been minimized.

Show comment
Hide comment
@StijnVrolijk

StijnVrolijk Oct 5, 2018

Contributor

@wouterverstuyf we have no plans for removing the current Thumbnails from Fork CMS.

Contributor

StijnVrolijk commented Oct 5, 2018

@wouterverstuyf we have no plans for removing the current Thumbnails from Fork CMS.

@carakas carakas added this to the 5.4.1 milestone Oct 16, 2018

@carakas carakas added the Has PR label Oct 16, 2018

@carakas carakas self-assigned this Oct 16, 2018

@carakas

This comment has been minimized.

Show comment
Hide comment
@carakas

carakas Oct 16, 2018

Member

Managed to solve it in slightly less lines of code :) #2660

Member

carakas commented Oct 16, 2018

Managed to solve it in slightly less lines of code :) #2660

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