BlackbitDigitalCommerce / pimcore-data-director

Import Bundle for Pimcore
16 stars 3 forks source link

Request for clarification and evaluation of the necessity of overwriting PreviewGeneratorReference in ClassChangedListener #138

Open R-O-BIN opened 8 months ago

R-O-BIN commented 8 months ago

Hello,

I have noticed that the ClassChangedListener currently overwrites the PreviewGeneratorReference every time a ClassDefinition is changed. I would like to have the exact purpose and necessity of this functionality clarified. Could you please provide some insight into its intended use and whether it is considered essential to the system? In addition, I am interested in understanding the implications and possible consequences of disabling the PreviewGeneratorReference setting. Would it be possible to assess whether this functionality is critical to the DataDirector or whether it can be safely disabled without any negative impact?

Thank you very much for your feedback.

R-O-BIN commented 8 months ago

@BlackbitDevs any answers?

R-O-BIN commented 2 months ago

@BlackbitDevs any answers? We pay a lot of money for the DataDirector so it would be nice to get an answer, because your EventListener is the reason for a lot of problems in our System.

olenakitova commented 2 months ago

Hello R-O-BIN,

Thank you for your request and apologies for the delay. We will review your request next week and get back to you. Could you please let us know which version of DataDirector you are using? Thank you in advance.

Kind regards,
The Blackbit Team

R-O-BIN commented 2 months ago

Hello @olenakitova, Thank you for your response. Currently, I use version 3.6, but we had the same issues with the previous versions.

BlackbitDevs commented 2 months ago

Hi @R-O-BIN,

there are 2 purposes of the preview generator:

  1. show clients that the "Preview" feature exists in Pimcore
  2. have a basic default implementation with
    1. all field values (similar to version view but bigger as there is no version grid panel)
    2. visualization of object dependencies (show context of the current object with option to jump directly to related elements)

I have noticed that the ClassChangedListener currently overwrites the PreviewGeneratorReference every time a ClassDefinition is changed. The preview generator service only gets added for newly created classes. If you do not want it, you can remove it and resave the class. In this case, it will not get added again.

Data Director's purpose is to improve Pimcore concerning all PIM functionalities. On the one hand this is imports, exports, automation but this is also about providing useful defaults for other PIM functionalitites - like the Preview in this case. We are currently creating a video series about different aspects of this second part, how Data Director improves Pimcore's default features, named Pimcore on Steroids to explain those features.

So short answer for you: You can safely remove @DataDirectorPreview from your class. But I would really love to get to know the reason - maybe we can find a better solution ;-)

DieserRxbin commented 2 months ago

Hi @BlackbitDevs ,

How was it decided that the DataDirector standard is applicable to all customers? We don't want a preview in some data objects and prefer using the preview provided by the LinkGenerator in others. A tabular preview is not a solution for our customers because they want to see the final appearance.

The EventListener sets the PreviewGeneratorReference every time the ClassDefinition changes. Such significant interventions in the ClassDefinition should be configurable.

For us, DataDirector is just an import and export tool, and we use it solely for these purposes. It shouldn't be Pimcore on steroids.

We noticed that you added the check for the LinkGeneratorReference in the EventListener, but it would be much better for us if the EventListener that changes something in the ClassDefinition could be made configurable.

Additionally, I noticed that under Navigation & Features in Documents, the DataDirector overrides the CSS class of the flags in the top right corner (Pimcore 10.6), leading to an incorrect representation. In our opinion, the DataDirector should maintain its core functionality of importing and exporting data without altering Pimcore's default functionality.

BlackbitDevs commented 2 months ago

How was it decided that the DataDirector standard is applicable to all customers? We don't want a preview in some data objects and prefer using the preview provided by the LinkGenerator in others. A tabular preview is not a solution for our customers because they want to see the final appearance.

As said, you can simply remove the preview generator service from your class.

The EventListener sets the PreviewGeneratorReference every time the ClassDefinition changes. Such significant interventions in the ClassDefinition should be configurable.

This is not correct. This gets only set for newly created classed:

    Blackbit\DataDirectorBundle\EventListener\ClassChangedListener:
        public: true
        tags:
            ...
            - { name: kernel.event_listener, event: pimcore.class.preAdd, method: addPreviewService }

Which DD version are you using?

Additionally, I noticed that under Navigation & Features in Documents, the DataDirector overrides the CSS class of the flags in the top right corner (Pimcore 10.6), leading to an incorrect representation.

Which CSS rule do you mean? In my opinion this is a Pimcore bug as the following rules have same specificity:

.pimcore_icon_language_{{ language|lower }} {
    background: url({{ iconFile }}) center center/contain no-repeat;
}

This implicitly sets the background-size to auto.

And

.pimcore_document_property_language_label {
  ...
  background-size: 16px 16px;
}

has same specificity. Whichever comes last, will get used.

In our opinion, the DataDirector should maintain its core functionality of importing and exporting data without altering Pimcore's default functionality.

Its core functionality is supporting PIM projects. But please report whenever a feature does not work for your use-case, then we will find a solution. But the preview generator can easily be removed.