Skip to content

Conversation

@igorsimovski
Copy link
Contributor

  • update json export for each
  • update FE to include the dataLayer in the markup

 - update json export for each
 - update FE to include the dataLayer in the markup
@igorsimovski igorsimovski requested a review from nhirrle March 4, 2022 15:48
</dependency>
<dependency>
<groupId>com.adobe.cq</groupId>
<artifactId>core.wcm.components.core</artifactId>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh we have now a hard dependency on core components, right? because of the interface & utility function?
I am just bit worried because this can conflict with a client core component version...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's discuss this in some call

Copy link
Collaborator

@nhirrle nhirrle Mar 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@igorsimovski : how much code would you need to clone of we don't add the dependency?

I'd like to avoid adding the dependency to core competents to not dictate that and which version needs to be used by the client

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought: can't this be overcome with a specific entry in the Import-Package section in the client project?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@igorsimovski : how much code would you need to clone of we don't add the dependency?
com.adobe.cq.wcm.core.components.models.datalayer.ComponentData (we use the getData method from this interface)
com.adobe.cq.wcm.core.components.internal.jackson.ComponentDataModelSerializer (this is the serializer for the field/'json property' associated with getData())
com.adobe.cq.wcm.core.components.util.ComponentUtils (to check if data layer is enabled in CA configs - This can be omitted either by making our own ca config or rely only on a dataLayer on/off switch in the components)
com.adobe.cq.wcm.core.components.models.datalayer.builder.DataLayerBuilder (this is a rather large implementation that offers a convenient way of building the dataLayer json, but we could do w/o it - we can simply do it with a dedicated POJO for each component)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought: can't this be overcome with a specific entry in the Import-Package section in the client project?

https://www.baeldung.com/maven-version-collision

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for my late reply. I think it makes sense to discuss this one to one

…nt_and_Search_Result_Tab

# Conflicts:
#	pom.xml
#	ui.frontend/src/main/webpack/site/js/types/searchOptions.ts
 - initiate DOMContentLoadedEvent after search query (to register tab data layers)
 - update index.html (tab urls)
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 9, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

40.9% 40.9% Coverage
0.0% 0.0% Duplication

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 this pull request may close these issues.

3 participants