mavicellc / aem-vue-editable-components

Apache License 2.0
11 stars 4 forks source link

Feature/grid related fixes #1

Closed anoack93 closed 3 years ago

anoack93 commented 3 years ago

Currently we are working on a nuxt integration for the aem spa editor which is based on your vue integration (thanks for your effort). Due to issues with the AEM Grid we created an internal fork of your package and fixed the missing grid css classes. However, you were a little bit faster and already released a fix yourself, but we added a few improvements in which you might be interessted, this includes:

I hope this helps you a little bit, keep up your good work!

If you like, we can also get in touch to work together on a better vue support.

anoack93 commented 3 years ago

8bd2ceb This achieves the same goal as the cqItemsOrder.map function call. The difference between this implementation and ours is that you are declaring a local variable to store the childComponents while we are just returning a new array with the map function.

I think the result is not exact the same. The Array.prototype.map function is designed to return an array with the same size like the original array. But in your code you are also apply some kind of filter. In some cases you are not having any return statement. I am not sure what map is doing in these cases but probably it will be equal to return undefined. So I think it is better to explicitly push elements into the array. This combines mapping and filtering and you don't have null or undefined values in your resulting array.

anoack93 commented 3 years ago

I will close this Pull Request because I think the changes are outdated.