-
Notifications
You must be signed in to change notification settings - Fork 2
feat: added cookie consent management link #22
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
Conversation
Signed-off-by: Asitha de Silva <[email protected]>
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 integrates Osano cookie consent management into the LFXFooter component by adding a “Manage cookie preferences” link, injecting the necessary Osano scripts, and updating styles.
- Declares and initializes the Osano global object for CMP interactions
- Appends a cookie preferences link that opens Osano’s drawer
- Adds CSS rules to style the link and hide the Osano widget
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/components/footer/footer.style.ts | Added .cookie-preferences styles for the new link |
| src/components/footer/footer.component.ts | Declared Osano types, inserted link HTML, and injected Osano scripts |
Comments suppressed due to low confidence (1)
src/components/footer/footer.component.ts:78
- New cookie preference UI and script-loading logic lack unit or integration tests; consider adding tests to cover link visibility toggling and Osano callback behavior.
<div class="cookie-preferences" part="cookie-preferences" style="display: none;">
|
Caution Review failedThe pull request is closed. WalkthroughThe footer component is updated to integrate Osano's cookie consent management drawer through a new "Manage cookie preferences" link. TypeScript declarations for the global Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FooterComponent
participant Window.Osano
User->>FooterComponent: Clicks "Manage cookie preferences" link
FooterComponent->>Window.Osano: Calls cm.showDrawer()
alt Osano available
Window.Osano-->>User: Displays consent management drawer
else Osano unavailable
FooterComponent-->>User: Logs warning
end
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✨ Finishing Touches
🧪 Generate Unit Tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
♻️ Duplicate comments (4)
src/components/footer/footer.style.ts (1)
80-85: Fix the duplicate focus selector.There's a duplicate
.cookie-preferences a:focusselector with an empty rule followed by the actual implementation. This creates invalid CSS and should be removed.Apply this diff to fix the duplicate selector:
-.cookie-preferences a:focus { - .cookie-preferences a:focus { outline: var(--lfx-footer-link-focus-outline, 2px solid currentColor); outline-offset: var(--lfx-footer-link-focus-offset, 2px); }src/components/footer/footer.component.ts (3)
78-80: Improve accessibility of cookie preferences link.The trailing period outside the
<a>tag may cause issues for screen readers and creates inconsistent punctuation handling.Apply this diff to move the period inside the link:
- <a href="#" class="cookie-preferences-link" part="cookie-preferences-link">Manage cookie preferences</a>. + <a href="#" class="cookie-preferences-link" part="cookie-preferences-link">Manage cookie preferences.</a>
8-17: Update TypeScript declaration to match actual Osano API.The current declaration treats
Osanoas an object, but based on the injected script (lines 154-161),Osanois actually a function with adataqueue property, similar to Google Analytics.Apply this diff to fix the TypeScript declaration:
declare global { interface Window { - Osano?: { - cm?: { - hideWidget?: () => void; - showDrawer?: () => void; - }; - }; + Osano?: { + (event: string, callback: (...args: any[]) => void): void; + data: any[]; + cm?: { + hideWidget?: () => void; + showDrawer?: () => void; + }; + }; } }
58-58: Restore proper attributeChangedCallback signature.The simplified signature breaks Web Component specification compliance. The callback must accept all three parameters even if unused.
Apply this diff to restore the proper signature:
- attributeChangedCallback(name: string): void { + attributeChangedCallback(name: string, oldValue: string | null, newValue: string | null): void { if (name === 'cookie-tracking' && this._rendered) { this._handleCookieTracking(); } }
🧹 Nitpick comments (1)
src/components/footer/footer.component.ts (1)
133-138: Consider using CSS classes instead of inline styles.While the current implementation works, using CSS classes aligns better with the component's styling approach and improves maintainability.
Consider this alternative approach:
private _showCookiePreferencesLink(): void { const cookiePreferences = this.shadowRoot?.querySelector('.cookie-preferences') as HTMLElement; if (cookiePreferences) { - cookiePreferences.style.display = 'inline'; + cookiePreferences.classList.add('visible'); } }And add this CSS rule to footer.style.ts:
.cookie-preferences.visible { display: inline; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/footer/footer.component.ts(7 hunks)src/components/footer/footer.style.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
`src/components/**/*`: Web components should be placed in src/components/.
src/components/**/*: Web components should be placed in src/components/.
src/components/footer/footer.style.tssrc/components/footer/footer.component.ts
`src/components/**/*`: All components must follow the prescribed structure: copyright header, import statements, JSDoc comments, class extending HTMLElement, constructor with shado...
src/components/**/*: All components must follow the prescribed structure: copyright header, import statements, JSDoc comments, class extending HTMLElement, constructor with shadow DOM setup, private template creation method, connectedCallback, disconnectedCallback, and component registration with IIFE.
Component class names must use PascalCase with an LFX prefix (e.g., LFXFooter).
Custom element tag names must use kebab-case with an lfx- prefix (e.g., lfx-footer).
CSS custom properties must use kebab-case with a --lfx- prefix.
Private methods and properties must use an underscore prefix (e.g., _handleClick, _template).
Always use { mode: 'open' } for shadow DOM instantiation.
Use the StylesheetManager utility for stylesheet handling, including constructible stylesheets with fallback and caching.
Check for shadowRoot existence before manipulating it.
Use template elements for DOM structure and cache template content to avoid recreation.
Document all components with comprehensive JSDoc comments, including @element, @summary, @description, @csspart, @cssproperty, @attr, @fires, and optionally @example.
src/components/footer/footer.style.tssrc/components/footer/footer.component.ts
`src/components/**/*.style.ts`: Separate styles into .style.ts files and export styles as template literal strings. Include :host styles for the component container and reset margi...
src/components/**/*.style.ts: Separate styles into .style.ts files and export styles as template literal strings.
Include :host styles for the component container and reset margins/padding and set box-sizing in :host *.
Support responsive design with media queries, high contrast mode, and reduced motion preferences.
Use CSS custom properties for all themeable values.
src/components/footer/footer.style.ts
🔇 Additional comments (4)
src/components/footer/footer.style.ts (1)
65-79: LGTM! Cookie preferences styling follows existing patterns.The CSS implementation correctly follows the established patterns for footer links, including consistent use of CSS custom properties, hover states, and the Osano widget hiding rule.
Also applies to: 87-89
src/components/footer/footer.component.ts (3)
105-121: LGTM! Cookie preferences event handling is well implemented.The event handling properly prevents default behavior, includes error handling, and provides appropriate console warnings when Osano is unavailable.
192-200: LGTM! Script injection with proper error handling.The script injection logic correctly handles both head and body fallback scenarios, includes comprehensive error handling, and dispatches appropriate custom events.
151-171: Review script injection security and implementation.The initialization script correctly sets up the Osano function queue, but there are security and implementation concerns:
- Security: Dynamic script injection should validate the script source
- Implementation: The widget hiding logic may execute before the widget is created
Please verify that:
- The script source is from a trusted domain
- The initialization timing doesn't cause race conditions
#!/bin/bash # Verify Osano script domain and check for Content Security Policy considerations echo "Checking Osano script domain..." curl -I "https://cmp.osano.com/16A0DbT9yDNIaQkvZ/d6ac078e-c71f-4b96-8c97-818cc1cc6632/osano.js?variant=two" 2>/dev/null | head -10 echo -e "\nSearching for CSP headers or meta tags in the codebase..." rg -i "content-security-policy|csp" --type html --type ts --type js -A 2 -B 1
Signed-off-by: Asitha de Silva <[email protected]>
This pull request enhances the
LFXFootercomponent by adding support for managing cookie preferences using the Osano consent management platform. Key changes include the addition of a "Manage cookie preferences" link, integration of Osano scripts, and styling updates for the new functionality.Cookie Management Integration:
src/components/footer/footer.component.ts, src/components/footer/footer.component.tsR7-R29)src/components/footer/footer.component.ts, [1] [2]src/components/footer/footer.component.ts, [1] [2]Styling Updates:
src/components/footer/footer.style.ts, src/components/footer/footer.style.tsR65-R90)Code Improvements:
attributeChangedCallbackto remove unused parameters for better readability and maintainability. (src/components/footer/footer.component.ts, src/components/footer/footer.component.tsL44-R58)