Instead of overwriting the minify_exclude value, append your css file to it.

[hostep]

Fixes #17

[hostep]

I found a more "stable" solution for this issue which is explained over here: https://magento.stackexchange.com/a/198480/2911, I feel like this is a better solution then what's in this PR because if you install other 3rd party modules which also define the minify_exclude config, you'll run into more problems.

I can take a look at changing this, but not sure when I will find some time, maybe during the weekend...

[sydekumf]

Hey, to be honest, I am not sure which solution is better, so I trust you, if you say, that the other one is better :-)

[hostep]

@sydekumf: I've added a new commit, which seems to fix the issues. Can you do a review? I tried to adhere to your code standards, but I might have missed something, feel free to let me know or fix yourself after merging :)
It would be appreciated if after testing & merging you could add a new version tag, so we can upgrade using composer to the latest version so we don't have to add some temporary fixes on our side.

This solution is better then my previously suggested solution, because if using multiple 3rd party modules which modify the setting using xml, only one module's configuration would win. With this change, it should merge it with the configuration which is set by Magento or other 3rd party modules and the chance of problems is much lower.

If you are interested, I'm working on a fix to get included in Magento 2.3 if accepted, to make the merging work properly using xml, but the structure of the xml will change a little bit: magento/magento2#13687

Oh: and big thanks for the module, a customer of ours is really happy with this!

[sydekumf]

So I look into this and I am still not sure how to proceed. In my opinion, the issue is solved in 2.2 as they increased the version dependency for tubalmartin/cssmin (magento/magento2#8552).
So basibcally we can remove /Magenerds_PageDesigner/css/pd-ui.css from merge_exclude, right? Why exactly should we exclude the tiny_mce directory?

[hostep]

Hi!

Ah, sorry, I didn't notice the version requirement of tubalmartin/cssmin was upped in Magento 2.2.0 to 4.1.0, thanks for the notification!
But in Magento 2.1.11, they still require version 2.4.8-p4, am I correct in understanding that this version will still cause issues? If yes, then you still need to exclude your css file from minifying, because I'm assuming that your module is still supported in Magento 2.1.x?

Then about the tiny_mce directory, you guys don't need to exclude it (I'm going to change the title of this PR, since it no longer makes sense), but the directory has to be excluded by Magento, otherwise the tinymce editors in the backend, in the product or category edit forms would no longer work when minifying css & js is enabled. So my suggested workaround, is to not overwrite the minify_exclude value with your css file, but just append it, so in the end there will be 2 regexes, where the first one comes from Magento itself, and the second one from your module:

/tiny_mce/
/Magenerds_PageDesigner/css/pd-ui.css

Hope this makes it a bit clearer? :)

Thanks!

[sydekumf]

Yes, the module should still support 2.1, if you look into our releases: https://github.com/Magenerds/PageDesigner/releases
PageDesigner <= 2.0.0 = Magento 2.1
PageDesigner >= 3.0.0 = Magento 2.2

So your change should be done in a separate branch, branched out of the tag 2.0.0 and then we do a 2.1.0 tag. As we don't need your change in 3.0.0, as this is compatible to M 2.2 and M 2.2 has the new tubalmartin version. Could you do this branch? Would be awesome :-)

Also, I saw, that in your fix, the tiny_mce directory is still missing or do I understand that not correctly?

[hostep]

@sydekumf: I'm afraid that's not so easy as you think, because version 3.0.x of your module also supports Magento 2.1, otherwise you would have to remove ^100.0.0 constraint from the magento/framework in the composer.json file
And if you now release a version 3.0.3 where that constraint is removed, it means that composer will still install 3.0.2 on Magento 2.1 versions, correct? (I haven't actually tested it out, so if you want to be certain, try to install various versions of your module on various versions of Magento using composer).

Regarding the tiny_mce directory, Magento itself defines it over here: https://github.com/magento/magento2/blob/2.2.2/app/code/Magento/Store/etc/config.xml#L29-L31
The point of this PR is to not let your module overwrite this setting, but to merge it with the one from Magento.

[sydekumf]

You're right, I removed the composer.json dependency to M2.1 in the latest versions. It is intended that newer versions of PadeDesigner are supported for M2.2. So everything >= 3.0.0 = M2.2.
Maybe you can do a separate branch out of the version tag 2.0.0 now and pull request that? That would be awesome!

[hostep]

Can I suggest another approach? You create a version 4.0.0 which is for M2.2, and leave 3.0.2 to be compatible with 2.1 (since it currently already is so). Otherwise people installing this module on Magento 2.1 using composer and don't specify a certain version will still get version 3.0.2, and that doesn't match with what you are saying above.

Anyway, for me to create a new PR on a different branch, you would first have to create a new branch for me to work on, otherwise I can't send in a PR against a different branch.

So if we want to do this properly you could create a branch "v3" (you can pick your own name) based on commit 719ad92, on which I can apply this PR. This branch is then meant for Magento 2.1. You can leave the master branch as is en we can remove the <minify_exclude> section on the master branch, then start tagging new versions starting with '4.0.0' and use that branch for Magento 2.2

But you'll need to remember that you'll have to maintain two branches from that point on.

Another approach could be that we add a check in this PR to determine which Magento version is being used, and only execute the code in this PR on Magento 2.1 and not 2.2 (if we want to be even more strict, we can try to check for the version of the tubalmartin/cssmin module instead of the Magento version, in case tubalmartin/cssmin also gets updated in a future version of Magento 2.1)

By using this approach you would only have to maintain a single branch which is compatible with both Magento 2.1 and 2.2

Just some ideas, let me know what you think. I understand this is pretty complicated stuff.

[sydekumf]

Yeah, you're totally right. It is complicated :-) And I appreciate your feedback to that! I think your proposal is the best right now, that we include checks which Magento version we are on (or tubalmartin version) and dependent on that we can act differently with the merging.
Would it be possible that you could include that check then? Would be awesome ;-)
Again: I appreciate your work and feedback to that, I think the extensions get better if different opinions get discussed, thanks!

[hostep]

@sydekumf: ok, I've updated my PR with something I believe works.

Steps to test:

1. Enable JS & CSS Minifying in the Magento configuration (I directly modifed the database values)
2. Put the shop in production mode, make sure the static deployment also runs without issues
3. Open a product edit form in the adminhtml of Magento

Tested on a Magento 2.2.2 installation, result:

• pd-ui.min.css file is requested in the Magento backend
• pd-ui.min.css file contains correctly minified code, for example: .pd-col.pd-col-4{width:calc((100%/12*4) - 10px)}
• no issues with wysiwyg editor on product edit form

Tested on a Magento 2.1.7 installation, result:

• pd-ui.css file is requested in the Magento backend
• pd-ui.cssfile contains non-minified code, for example: .pd-col.pd-col-4 { width: calc((100% / 12 * 4) - 10px); }
• no issues with wysiwyg editor on product edit form

---

I wanted to find something simple to test the version of tubalmartin/cssmin without having to parse the composer.lock file for example, so I've opted to check for the existence of a certain class name. Luckily the library changed its class names and namespace structure when going from version 2 to version 3, so that was a really easy check to add.

So if it detects that tubalmartin/cssmin version 2 is in use, it will exclude the pd-ui.css file from minifying, and if it doesn't detect that version, or the library is not installed at all, nothing happens.

I think this should be pretty robust.

Feel free to test and/or give feedback!

[sydekumf]

Than

[hostep]

Fixed, good catch!

[sydekumf]

Great work! Thanks for your PR, we appreciate any new ideas :-)

[hostep]

Thanks @sydekumf: just verified version 3.1.0 on a production install with Magento 2.2.2 and it all looks ok, no problems anymore!

Thanks for the collaboration!