Skip to content
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

Image formats defined inside of theme directory are ignored #16

Open
almare opened this issue Nov 7, 2019 · 12 comments · May be fixed by #34
Open

Image formats defined inside of theme directory are ignored #16

almare opened this issue Nov 7, 2019 · 12 comments · May be fixed by #34

Comments

@almare
Copy link

almare commented Nov 7, 2019

Q A
Bug? yes
New Feature? no
Sulu Version 2.0
Browser Version Browser name and version

Actual Behavior

At the moment the image-format.xml inside path/to//config/image-formats.xml is ignored.

Expected Behavior

As written inside the docu I would expect that the path is working.

Steps to Reproduce

Add file to path/to//config/image-formats.xml

Possible Solutions

If have extended the ImageFormatCompilerPass inside theme Bundle to load the theme path. But this is not enough, because the preview inside the admin also not working. I need to add a image-format.xml inside src/Resources/config/image-formats.xml to get the admin preview working.

@axzx
Copy link

axzx commented Nov 8, 2019

you have to clear cache

@almare
Copy link
Author

almare commented Nov 8, 2019

Thank you for your reply. I did it x-times and including rm var/cache. It started working afterchanging this code inside theme bundle.

class ImageFormatCompilerPass extends AbstractImageFormatCompilerPass
{
    /**
     * {@inheritdoc}
     */
    protected function getFiles(ContainerBuilder $container)
    {
        $files = [];

        $activeTheme = $container->get('liip_theme.active_theme');
        $bundles = $container->getParameter('kernel.bundles');
        $configPath = 'config/image-formats.xml';
        $rootDir = $container->getParameter('kernel.project_dir') ;
        foreach ($activeTheme->getThemes() as $theme) {

            $themePath =  $rootDir .'/themes/'. $theme .'/'. $configPath;
            if(file_exists($themePath)){
                $files[] = $themePath;
            }

            foreach ($bundles as $bundleName => $bundle) {
                $bundleReflection = new \ReflectionClass($bundle);
                $path = sprintf(
                    '%s/Resources/themes/%s/%s',
                    dirname($bundleReflection->getFileName()),
                    $theme,
                    $configPath
                );

                if (file_exists($path)) {
                    $files[] = $path;
                }
            }
        }
        return $files;
    }
}

So it has nothing to do with the cache. I think.

@alexander-schranz
Copy link
Member

alexander-schranz commented Nov 8, 2019

Seems like ImageFormatCompilerPass currently doesn't support themes inside projects and only supports bundles.

@sulu/core-team where should we store theme specific configurations in a project. I would think about something like config/themes/<theme_name>/image-formats.xml?

@almare you can still define all your image formats in the default config/image-formats.xml.

@almare
Copy link
Author

almare commented Nov 8, 2019

root/config/image-formats.xml did also not work for me.

@danrot
Copy link
Contributor

danrot commented Nov 11, 2019

@alexander-schranz The path you have suggested makes sense to me 👍

@almare
Copy link
Author

almare commented Dec 12, 2019

Is this not part of the milestone 2.0?

@danrot
Copy link
Contributor

danrot commented Dec 12, 2019

I am open to accept a PR which fixes this, but since you should still be able to define your image formats in config/image-formats.xml, this has not a very high priority to me. Especially since the end result would be exactly the same, because the system merges all available image-formats anyway.

However, what bugs me more is that you've said that this is also not working for you. Did you add this to the config/image-formats.xml file that should already exist in your application beforehand and cleared the cache afterwards?

And I just realized that this might also be related to sulu/sulu#2639. If we allow to split the image-formats.xml file into multiple, and implement that in a way that the SuluThemeBundle just has to prepend some configuration, then we could even get rid of the CompilerPass in here.

@alexander-schranz And I am right with my assumption, that this issue should actually go into the SuluThemeBundle? If I see this correctly, this issue has to be fixed there.

@danrot
Copy link
Contributor

danrot commented Dec 12, 2019

@almare Oh, and where in the docs did you find some text that explains this should be possible? Just looked it up, and I couldn't find it.

@alexander-schranz alexander-schranz transferred this issue from sulu/sulu Dec 12, 2019
@SebTM
Copy link

SebTM commented Oct 1, 2021

I came across this issue today and figured it out in the code that it's not intended like I understood it - so I came here to report/discuss about changing the docs or maybe a PR:

https://docs.sulu.io/en/latest/book/image-formats.html

Or when you use the SuluThemeBundle you can define the formats in your theme folder:
path/to//config/image-formats.xml

combined with

By default, the bundle seeks for the themes in the %kernel.project_dir%/themes directory and looks for a theme configuration file named composer.json. This can be changed via the yaml configuration:

from the documentation here made me assume that "themes//config/image-formats.xml" would be a valid option as well. I'm open to provide a PR extending the current compiler-pass or adding a second one for this :)

@niklasnatter
Copy link
Contributor

Do I understand this right the path path/to/<theme>/config/image-formats.xml does not work at the moment?
If yes, it would be great if you could create a pull request for this!

@niklasnatter niklasnatter changed the title image-formats not working for themes Image formats defined inside of theme directory are ignored Oct 4, 2021
@NeuralClone
Copy link

NeuralClone commented Jun 16, 2022

I recently started working with this bundle and trying to get this particular feature to work as described led me here.

Unless I'm missing something obvious, I can confirm that path/to/<theme>/config/image-formats.xml doesn't work at the moment. That directory is ignored by the compiler pass as it's currently written. It iterates through my themes but it only adds image formats from bundles.

Since themes found in %kernel.project_dir%/themes are correctly showing up as installed themes at this point in the code (along with other themes added to the repository), it may not require much to get this working.

@websmurf websmurf linked a pull request Oct 31, 2022 that will close this issue
@websmurf
Copy link

websmurf commented Oct 31, 2022

Had the same issue on my environment, created a pull request that fixes the issue for me: #34

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants