-
Notifications
You must be signed in to change notification settings - Fork 481
refactor(sdk-analytics): Consolidate analytics data attributes and simplify Contentlet component #33948
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
base: main
Are you sure you want to change the base?
Conversation
…attributes ### Summary This commit refactors the analytics SDK by consolidating the import of analytics-related constants and updating data attribute names for consistency across the codebase. ### Changes Made - Moved `ANALYTICS_WINDOWS_ACTIVE_KEY` and `ANALYTICS_WINDOWS_CLEANUP_KEY` imports to a shared constants file for better organization. - Updated data attribute names in the impression tracker and identity tracker to use a consistent naming convention (`dotIdentifier`, `dotInode`, etc.). - Adjusted the contentlet component to apply the new class name for contentlet elements. ### Benefits - Improved code maintainability and readability by centralizing constant definitions. - Enhanced consistency in data attributes used across different components and plugins.
… impression tracker tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR consolidates analytics-related constants and data attributes to simplify the architecture across SDK packages. The main goal is to eliminate duplicate analytics logic in the React Contentlet component by standardizing on UVE's data attributes (data-dot-*) instead of maintaining separate analytics-specific attributes (data-dot-analytics-*).
Key changes:
- Moved analytics window constants from
@dotcms/uve/internalto@dotcms/analyticsfor better separation of concerns - Standardized data attribute names to use UVE's
data-dot-*pattern across all analytics plugins - Removed conditional analytics state management from React's Contentlet component (now always renders UVE attributes)
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
core-web/libs/sdk/uve/src/lib/dom/dom.utils.ts |
Removed getDotAnalyticsAttributes() helper function (analytics now uses standard UVE attributes) |
core-web/libs/sdk/uve/src/lib/core/core.utils.ts |
Removed isAnalyticsActive() utility function (no longer needed for conditional rendering) |
core-web/libs/sdk/uve/src/lib/core/core.spec.ts |
Removed 50+ lines of tests for deleted isAnalyticsActive() function |
core-web/libs/sdk/uve/src/internal/constants.ts |
Removed analytics constants ANALYTICS_WINDOWS_ACTIVE_KEY and ANALYTICS_WINDOWS_CLEANUP_KEY (moved to analytics SDK) |
core-web/libs/sdk/react/src/lib/next/hooks/useIsAnalyticsActive.ts |
Deleted entire hook file (76 lines) - no longer needed for conditional rendering |
core-web/libs/sdk/react/src/lib/next/components/Contentlet/Contentlet.tsx |
Simplified component by removing analytics state checks and always applying UVE attributes; added CONTENTLET_CLASS constant |
core-web/libs/sdk/react/src/lib/next/__test__/components/Contentlet.test.tsx |
Updated tests to reflect simplified logic - removed analytics mocks and updated assertions |
core-web/libs/sdk/analytics/src/lib/core/shared/constants/dot-analytics.constants.ts |
Added ANALYTICS_WINDOWS_ACTIVE_KEY, ANALYTICS_WINDOWS_CLEANUP_KEY, and CONTENTLET_CLASS constants |
core-web/libs/sdk/analytics/src/lib/core/shared/utils/dot-analytics.utils.ts |
Updated attribute reading to use standard data-dot-* names instead of data-dot-analytics-* |
core-web/libs/sdk/analytics/src/lib/core/plugin/impression/dot-analytics.impression-tracker.ts |
Updated to use data-dot-dom-index instead of data-dot-analytics-dom-index |
core-web/libs/sdk/analytics/src/lib/core/plugin/impression/dot-analytics.impression-tracker.spec.ts |
Updated test fixtures to use new attribute names and CONTENTLET_CLASS |
core-web/libs/sdk/analytics/src/lib/core/plugin/identity/dot-analytics.identity.activity-tracker.ts |
Updated imports to reference constants from analytics SDK instead of UVE |
core-web/libs/sdk/analytics/src/lib/core/plugin/identity/dot-analytics.identity.activity-tracker.spec.ts |
Updated imports to reference constants from analytics SDK instead of UVE |
core-web/libs/sdk/analytics/src/lib/core/dot-analytics.content.ts |
Updated imports to reference constants from analytics SDK instead of UVE |
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
core-web/libs/sdk/analytics/src/lib/core/shared/constants/dot-analytics.constants.ts
Show resolved
Hide resolved
core-web/libs/sdk/react/src/lib/next/components/Contentlet/Contentlet.tsx
Show resolved
Hide resolved
nicobytes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DotAnalyticsAttributes type definition still references the old attribute naming convention:
type AnalyticsAttribute<T extends string> = `data-dot-analytics-${T}`;
export type DotAnalyticsAttributes = {
[K in AnalyticsAttributeKey as AnalyticsAttribute<K>]: string;
};|
In
|
Summary
Consolidates analytics data attributes by moving constants from UVE to sdk-analytics and simplifies the sdk-react
Contentletcomponent by removing redundant analytics state management. Analytics attributes now always render using UVE's standard contentlet attributes instead of separate conditional logic.Changes Made
SDK Analytics (
libs/sdk/analytics)Moved analytics constants from
@dotcms/uve/internaltosdk-analytics/constants:ANALYTICS_WINDOWS_ACTIVE_KEY- Analytics active state flagANALYTICS_WINDOWS_CLEANUP_KEY- Analytics cleanup function referenceCONTENTLET_CLASS- Renamed fromANALYTICS_CONTENTLET_CLASStodotcms-contentletUpdated data attribute naming to use standard UVE attributes:
data-dot-analytics-identifier→data-dot-identifierdata-dot-analytics-inode→data-dot-inodedata-dot-analytics-contenttype→data-dot-typedata-dot-analytics-title→data-dot-titledata-dot-analytics-basetype→data-dot-basetypedata-dot-analytics-dom-index→data-dot-dom-indexUpdated all analytics plugins to use consolidated constants and new attribute names:
SDK React (
libs/sdk/react)Removed
useIsAnalyticsActivehook (useIsAnalyticsActive.ts)Simplified
Contentletcomponent (Contentlet.tsx:55-78):analyticsAttributesuseMemocontainerClassNameuseMemoisAnalyticsActivestate checkgetDotContentletAttributes()CONTENTLET_CLASS(dotcms-contentlet)Updated tests (Contentlet.test.tsx):
useIsAnalyticsActivemockgetDotAnalyticsAttributesmockSDK UVE (
libs/sdk/uve)getDotAnalyticsAttributes()function (analytics now uses standard UVE attributes)isAnalyticsActive()function (no longer needed)ANALYTICS_WINDOWS_ACTIVE_KEYandANALYTICS_WINDOWS_CLEANUP_KEYconstants (moved to sdk-analytics)Technical Details
Problem: The sdk-react
Contentletcomponent had duplicate analytics attribute logic that conditionally rendered based onisAnalyticsActive && !isDevMode. This created complexity and separated analytics attributes from UVE's standard contentlet attributes.Solution:
@dotcms/analyticswhere they belonggetDotContentletAttributes()which already provides all necessary data attributesdata-dot-*attributesdotcms-contentlet(used by both UVE and analytics)Benefits:
Breaking Changes
CSS Class Change: The analytics contentlet class changed from
dotcms-analytics-contentlettodotcms-contentlet. This aligns with UVE's standard class and should not affect existing implementations as both classes serve the same selector purpose.Data Attribute Changes: Analytics data attributes now use standard UVE naming (
data-dot-*instead ofdata-dot-analytics-*). The analytics SDK internally handles reading these attributes, so no client-side changes are needed.Testing
ContentletcomponentRelated Issues
Closes #33947
Additional Notes
This refactoring aligns with the strategy to consolidate analytics functionality and reduce code duplication across the SDK. Future enhancements can build on this simplified foundation without managing separate analytics attribute logic.
The change ensures analytics tracking is always active and consistent, removing edge cases where analytics might be disabled outside Edit Mode.