• src/blocks/sticky-menu/index.js (6)
• src/blocks/sticky-menu/inspector.js (1)
• src/blocks/sticky-menu/render.php (1)
• src/blocks/sticky-menu/style.scss (7)
• src/blocks/sticky-menu/view.js (22)

• build/blocks/sticky-menu/block.json
• src/blocks/sticky-menu/block.json

• src/blocks/sticky-menu/view.js (diff tokens exceeds limit)
• src/blocks/sticky-menu/index.js (nothing obtained from openai)
• src/blocks/sticky-menu/style.scss (nothing obtained from openai)
• src/blocks/sticky-menu/inspector.js (nothing obtained from openai)
• src/blocks/sticky-menu/render.php (nothing obtained from openai)

• src/blocks/sticky-menu/render.php (no response)
• src/blocks/sticky-menu/inspector.js (no response)
• src/blocks/sticky-menu/style.scss (no response)
• src/blocks/sticky-menu/index.js (no response)
• src/blocks/sticky-menu/view.js (no response)

• Review: 0
• LGTM: 0

Chat with CodeRabbit Bot (@coderabbitai)

• Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
• Invite the bot into a review comment chain by tagging @coderabbitai in a reply.

Code suggestions

• The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
• You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

• Add @coderabbitai: ignore anywhere in the PR description to pause further reviews from the bot.

src/blocks/sticky-menu/index.js (2)

26-38: Keep the Edit docblock in sync with the function signature.

Edit now accepts { attributes, setAttributes, clientId }, but the docblock only documents attributes and setAttributes. Please add a @param {string} props.clientId entry so the docs match the actual props.

---

141-147: Avoid javascript:void(0); in the editor anchor href.

For the editor preview button, href="javascript:void(0);" is unnecessary and generally discouraged. Consider mirroring the frontend and using href={#${item.id}} (or at least '#') while relying on your JS to call preventDefault() where appropriate.

src/blocks/sticky-menu/style.scss (1)

1-5: Reconsider !important on the wrapper padding.

The new wrapper rule forces padding: 0 !important;, while your JS already removes padding via wrapperStyle. To stay in line with the “avoid !important” guideline and keep specificity manageable, it would be better to drop !important here unless you have a concrete conflict that cannot be resolved otherwise.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

• src/blocks/sticky-menu/view.js (22)

• src/blocks/sticky-menu/view.js (diff tokens exceeds limit)

• src/blocks/sticky-menu/view.js (no response)

• Review: 0
• LGTM: 0

Chat with CodeRabbit Bot (@coderabbitai)

• Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
• Invite the bot into a review comment chain by tagging @coderabbitai in a reply.

Code suggestions

• The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
• You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

• Add @coderabbitai: ignore anywhere in the PR description to pause further reviews from the bot.

src/blocks/sticky-menu/view.js (6)

155-235: Tidy up unused context parameter and comment punctuation in add_mobile_header.

• The context argument is constructed in initialize_mobile_sections but not used in add_mobile_header. Either wire it into the header (e.g. for labels/analytics) or remove it to avoid dead parameters.
• The inline comment on Line 224 (// Always collapse on Escape if expanded) is missing a trailing full stop, which is required by the stated commenting guideline.

Example diff for the unused parameter (if you opt to remove it):

-lsx_to.add_mobile_header = function (section, context) {
+lsx_to.add_mobile_header = function (section) {
@@
-        const context = {
-            section_id: section.id,
-            section_title: section_title,
-            is_collapsed: true
-        };
+        // Context no longer needed here; section metadata can be read directly when required.

And for the comment punctuation:

-                    // Always collapse on Escape if expanded
+                    // Always collapse on Escape if expanded.

---

334-422: Guard IntersectionObserver usage and consider mobile/resize behaviour.

The IntersectionObserver‑based scroll spy is a good performance optimisation, but there are two edge cases worth tightening up:

• Feature detection: If this script runs in an environment without IntersectionObserver, new IntersectionObserver(...) will throw. Even if you only target modern browsers, a defensive guard is low‑cost.

• Resize from desktop to mobile: When the page starts on desktop, the observer remains active even after check_mobile switches to mobile mode. This means the observer callback can still update current_section and active menu state on mobile, contrary to the doc comment that scroll spy is “desktop only”.

A minimal hardening could look like:

-    // Initialize Intersection Observer for scroll spy (desktop only)
-    if (!lsx_to.sticky_menu.is_mobile) {
+    // Initialize Intersection Observer for scroll spy (desktop only, where supported).
+    if (!lsx_to.sticky_menu.is_mobile && typeof window.IntersectionObserver !== 'undefined') {
@@
-            lsx_to.sticky_menu.observer = new IntersectionObserver(function (entries) {
+            lsx_to.sticky_menu.observer = new IntersectionObserver(function (entries) {
+                if (lsx_to.sticky_menu.is_mobile) {
+                    return;
+                }
@@
             }, observer_options);

If you need to support starting on mobile then resizing up, you may also want to (re)create or reconnect the observer inside check_mobile when transitioning from mobile to desktop.

---

457-497: Attach handlers to the actual button element and improve ARIA/i18n for menu items.

The current implementation binds click/keydown handlers to the .lsx-to-sticky-menu-item container, while the interactive control is .lsx-to-sticky-menu-button. This works via event bubbling but is less precise for accessibility and future refactors. Also, the ARIA label string is hard‑coded in English.

Consider:

• Targeting the button where present to keep semantics aligned and avoid surprises if container markup changes.
• Adding event.stopPropagation() in the handlers if the menu ever sits inside another clickable container, to avoid double navigation.
• Making the ARIA label translatable, using your existing localisation mechanism rather than a hard‑coded English string.

Illustrative refactor:

-    menu_items.forEach(function (item) {
-        // Enhance button accessibility
-        const button = item.querySelector('.lsx-to-sticky-menu-button');
-        if (button) {
-            const section_id = item.getAttribute('data-section-id');
-            const section_title = button.textContent.trim();
-
-            // Add descriptive aria-label
-            button.setAttribute('aria-label', `Navigate to ${section_title} section`);
-            // Add role and aria-current for active state
-            button.setAttribute('role', 'tab');
-            button.setAttribute('aria-current', 'false');
-        }
-
-        item.addEventListener('click', function (event) {
+    menu_items.forEach(function (item) {
+        const button = item.querySelector('.lsx-to-sticky-menu-button') || item;
+        const section_id = item.getAttribute('data-section-id');
+        const section_title = button.textContent.trim();
+
+        // Add descriptive aria-label (ideally from a localised string).
+        button.setAttribute('aria-label', `Navigate to ${section_title} section`);
+        button.setAttribute('role', 'tab');
+        button.setAttribute('aria-current', 'false');
+
+        button.addEventListener('click', function (event) {
             event.preventDefault();
-            const section_id = this.getAttribute('data-section-id');
-            if (section_id) {
-                lsx_to.scroll_to_section(section_id);
-            }
+            event.stopPropagation();
+            if (section_id) {
+                lsx_to.scroll_to_section(section_id);
+            }
         });
@@
-        item.addEventListener('keydown', function (event) {
-            const section_id = this.getAttribute('data-section-id');
+        button.addEventListener('keydown', function (event) {
             if (section_id && (event.key === 'Enter' || event.key === ' ')) {
                 event.preventDefault();
+                event.stopPropagation();
                 lsx_to.scroll_to_section(section_id);
             }
         });

---

576-589: Optional: also clean up the resize listener if re‑initialisation on the same page is expected.

cleanup_sticky_menu removes the scroll listener and disconnects the observer, which covers the main leak risks. If this block can be initialised/cleaned up multiple times on a long‑lived page (AJAX navigation, partial reloads), you may also want to remove the resize listener that initialize_scroll_spy adds, by storing the check_mobile reference on lsx_to.sticky_menu and calling window.removeEventListener('resize', ...) here.

---

600-625: Localise the screen‑reader announcement string.

The announcement message (Navigated to ${section_title} section) is currently hard‑coded in English. To align with WordPress localisation practices, consider sourcing this base phrase from your translation mechanism so that ARIA messages are available in all supported languages.

---

676-690: General: DOM ready/unload wiring is fine; consider normalising inline comment punctuation.

The DOMContentLoaded hook to initialize_sticky_menu and the beforeunload hook to cleanup_sticky_menu are straightforward and match the lifecycle described in the docblocks.

One minor style point: several single‑line comments in this file (for example on Lines 54, 64, 73, 80, 88, 91, 111, 118, 128, 131, 163, 169, 188, 194, 199, 207, 213, 224, 372, 399, 404, 420, 533, 556, 577, 586) do not end with a full stop, which your guidelines call for. It might be worth normalising these in a follow‑up pass for consistency.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

• src/blocks/sticky-menu/index.js (6)
• src/blocks/sticky-menu/style.scss (7)

• src/blocks/sticky-menu/style.scss (nothing obtained from openai)
• src/blocks/sticky-menu/index.js (nothing obtained from openai)

• src/blocks/sticky-menu/style.scss (no response)
• src/blocks/sticky-menu/index.js (no response)

• Review: 0
• LGTM: 0

Chat with CodeRabbit Bot (@coderabbitai)

• Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
• Invite the bot into a review comment chain by tagging @coderabbitai in a reply.

Code suggestions

• The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
• You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

• Add @coderabbitai: ignore anywhere in the PR description to pause further reviews from the bot.

src/blocks/sticky-menu/style.scss (4)

66-67: Use colour variables instead of hardcoded rgba fallbacks.

Lines 66–67 and 71–72 use hardcoded rgba(0, 0, 0, 0.1) and #fff fallbacks instead of leveraging the colour variables defined in src/content-helper/common/css/variables.scss. Define semantic colour variables (e.g., --hover-overlay-color, --button-text-light) and use them consistently throughout.

 .lsx-to-sticky-menu-button:hover {
-	background-color: var(--hover-bg-color, rgba(0, 0, 0, 0.1));
-	color: var(--hover-text-color, inherit);
+	background-color: var(--hover-bg-color, var(--hover-overlay-color, rgba(0, 0, 0, 0.1)));
+	color: var(--hover-text-color, inherit);
 }

 .lsx-to-sticky-menu-button.active {
-	background-color: var(--active-bg-color, currentColor);
-	color: var(--active-text-color, var(--wp--preset--color--background, #fff));
+	background-color: var(--active-bg-color, var(--active-overlay-color, currentColor));
+	color: var(--active-text-color, var(--wp--preset--color--background));
 }

Also applies to: 71-72

---

116-116: Use colour variables for border colours instead of hardcoded values.

Lines 116, 141, and 184 use hardcoded rgba(0, 0, 0, 0.1) for borders. Define a semantic border-colour variable (e.g., --border-subtle-color) in variables.scss and reference it throughout to maintain consistency and enable theme-wide adjustments.

 .lsx-to-sticky-menu-section-wrapper {
-	border-bottom: 1px solid rgba(0, 0, 0, 0.1);
+	border-bottom: 1px solid var(--border-subtle-color, rgba(0, 0, 0, 0.1));

Also applies to: 141-141, 184-184

---

176-176: Remove duplicate padding declaration.

The padding: 1rem; property is declared twice in the button.lsx-to-section-header rule (lines 176 and 185). Keep only one declaration to improve code clarity and maintainability.

 button.lsx-to-section-header {
 	background: none;
 	border: none;
 	cursor: pointer;
 	font-family: inherit;
 	font-size: inherit;
 	color: inherit;
 	text-decoration: none;
 	transition: all 0.2s ease;
 	padding: 1rem;
 	border-radius: 0;
 	width: 100%;
 	display: none;
 	align-items: center;
 	justify-content: space-between;
 	text-align: left;
 	font-weight: 600;
 	border-bottom: 1px solid rgba(0, 0, 0, 0.1);
-	padding: 1rem;

Also applies to: 185-185

---

114-114: Consider using em units for media query breakpoints for consistency.

Line 114 uses max-width: 767px for the mobile breakpoint. For improved accessibility and consistency with modern practices, consider using em units (e.g., 48em ≈ 768px when base font size is 16px). This ensures the breakpoint scales with user-defined font sizes.

-@media (max-width: 767px) {
+@media (max-width: 48em) {

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

• src/blocks/sticky-menu/style.scss (7)

• src/blocks/sticky-menu/style.scss (nothing obtained from openai)

• src/blocks/sticky-menu/style.scss (no response)

• Review: 0
• LGTM: 0

Chat with CodeRabbit Bot (@coderabbitai)

• Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
• Invite the bot into a review comment chain by tagging @coderabbitai in a reply.

Code suggestions

• The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
• You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

• Add @coderabbitai: ignore anywhere in the PR description to pause further reviews from the bot.

src/blocks/sticky-menu/view.js:495

• The PR description mentions implementing event.stopPropagation() to prevent browser default scrolling behavior for keyboard navigation, and adding an is_scrolling_programmatically flag to prevent scroll spy interference. However, these features are not present in the updated view.js file. The keyboard event handlers at lines 489-495 only call event.preventDefault(), but don't include event.stopPropagation() or any programmatic scrolling flag management. This means the described keyboard navigation fixes may not be fully implemented.

        item.addEventListener('keydown', function (event) {
            const section_id = this.getAttribute('data-section-id');
            if (section_id && (event.key === 'Enter' || event.key === ' ')) {
                event.preventDefault();
                lsx_to.scroll_to_section(section_id);
            }
        });

src/blocks/sticky-menu/view.js:495

• Event listeners are being added to the .lsx-to-sticky-menu-item container (lines 480-495), but according to the PR description, these should be attached directly to the .lsx-to-sticky-menu-button elements for proper keyboard navigation. The current implementation attaches click and keydown handlers to the parent <li> element rather than the <a> button element, which could cause issues with keyboard focus and event handling. The button is already retrieved at line 467 for accessibility enhancements - the event listeners should be attached to it instead.

        item.addEventListener('click', function (event) {
            event.preventDefault();
            const section_id = this.getAttribute('data-section-id');
            if (section_id) {
                lsx_to.scroll_to_section(section_id);
            }
        });

        // Add keyboard support for menu items
        item.addEventListener('keydown', function (event) {
            const section_id = this.getAttribute('data-section-id');
            if (section_id && (event.key === 'Enter' || event.key === ' ')) {
                event.preventDefault();
                lsx_to.scroll_to_section(section_id);
            }
        });

src/blocks/sticky-menu/index.js:41

• Unused variable backgroundColor.

    backgroundColor,

src/blocks/sticky-menu/index.js:42

• Unused variable textColor.

    textColor,

src/blocks/sticky-menu/index.js:191

• Unused variable backgroundColor.

    backgroundColor,

src/blocks/sticky-menu/index.js:192

• Unused variable textColor.

    textColor,

• src/blocks/sticky-menu/view.js (22)

• src/blocks/sticky-menu/view.js (diff tokens exceeds limit)

• src/blocks/sticky-menu/view.js (no response)

• Review: 0
• LGTM: 0

Chat with CodeRabbit Bot (@coderabbitai)

• Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
• Invite the bot into a review comment chain by tagging @coderabbitai in a reply.

Code suggestions

• The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
• You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

• Add @coderabbitai: ignore anywhere in the PR description to pause further reviews from the bot.

src/blocks/sticky-menu/view.js (1)

155-235: Tidy add_mobile_header: unused context param and comment punctuation.

• The context parameter is never used in this function, but is documented in the JSDoc. This is likely to trigger no-unused-vars in the WordPress ESLint config. Either remove the parameter (and update the JSDoc and caller in initialize_mobile_sections) or use it meaningfully (for example, to derive accessible names or IDs).
• Several newly added line comments here (for example lines 159, 163, 188, 199, 205) and in the Escape branch comment below do not end with a full stop. The coding guidelines you supplied require each line comment to end with a period; please update them accordingly.

Example for the Escape branch:

-                    // Always collapse on Escape if expanded
+                    // Always collapse on Escape if expanded.

src/blocks/sticky-menu/view.js (8)

50-97: scroll_to_section logic looks sound; consider sharing offset calculation.

The scrolling, reduced‑motion handling, and focus management are solid. You now compute admin‑bar/masthead/sticky‑menu offsets here, in get_active_section_on_scroll, and in initialize_scroll_spy; extracting a small helper (e.g. getStickyOffset()) would keep the behaviour consistent and make future design‑token changes safer.

---

108-143: Use braces for the early return in toggle_mobile_section for clarity.

if (!element) return; is terse and out of step with the rest of the file and WordPress JS style, which prefers explicit blocks.

-    if (!element) return;
+    if (!element) {
+        return;
+    }

This improves readability without changing behaviour.

---

246-260: update_active_menu_item is global; consider scoping for multiple sticky menus.

This function currently queries .wp-block-lsx-tour-operator-sticky-menu .lsx-to-sticky-menu-button across the whole document and activates only the first button whose data-section-id matches. If multiple sticky‑menu blocks can exist on a page, one menu’s scroll state will clear the active state on all others.

Consider scoping the query to a specific sticky‑menu container (passed in or stored in state) so that each instance manages its own active button set independently.

---

271-422: Scroll‑spy and IntersectionObserver offsets are fine; consider a shared helper.

The scroll‑spy logic and IntersectionObserver configuration look correct and defensive (offsets include admin bar, masthead, and sticky menu; desktop/mobile switching is guarded). However, offset computation is now duplicated across scroll_to_section, get_active_section_on_scroll, and initialize_scroll_spy.

A small shared helper such as lsx_to.get_sticky_header_offset() would reduce drift and make it easier to keep behaviour aligned with future header or design‑system changes.

---

457-497: Prefer binding events to the actual button and consider stopping propagation.

Handlers are currently attached to .lsx-to-sticky-menu-item (the container) and then look up the inner .lsx-to-sticky-menu-button. For clearer semantics and keyboard behaviour, consider attaching both click and keydown handlers directly to the button element instead, and optionally stopping propagation to avoid interfering with outer handlers.

For example:

-    menu_items.forEach(function (item) {
-        // Enhance button accessibility
-        const button = item.querySelector('.lsx-to-sticky-menu-button');
-        if (button) {
-            const section_id = item.getAttribute('data-section-id');
-            const section_title = button.textContent.trim();
+    menu_items.forEach(function (item) {
+        const button = item.querySelector('.lsx-to-sticky-menu-button');
+        if (button) {
+            const section_id = item.getAttribute('data-section-id');
+            const section_title = button.textContent.trim();
@@
-        item.addEventListener('click', function (event) {
-            event.preventDefault();
-            const section_id = this.getAttribute('data-section-id');
-            if (section_id) {
-                lsx_to.scroll_to_section(section_id);
-            }
-        });
+        button.addEventListener('click', function (event) {
+            event.preventDefault();
+            event.stopPropagation();
+            if (section_id) {
+                lsx_to.scroll_to_section(section_id);
+            }
+        });
@@
-        item.addEventListener('keydown', function (event) {
-            const section_id = this.getAttribute('data-section-id');
-            if (section_id && (event.key === 'Enter' || event.key === ' ')) {
-                event.preventDefault();
-                lsx_to.scroll_to_section(section_id);
-            }
-        });
+        button.addEventListener('keydown', function (event) {
+            if (section_id && (event.key === 'Enter' || event.key === ' ')) {
+                event.preventDefault();
+                event.stopPropagation();
+                lsx_to.scroll_to_section(section_id);
+            }
+        });

This better aligns with the intent to have event listeners on the actual interactive control.

---

576-589: Consider removing the resize listener in cleanup_sticky_menu for completeness.

cleanup_sticky_menu removes the scroll listener and disconnects the IntersectionObserver, but the resize listener registered in initialize_scroll_spy is left in place. On normal full page loads this is harmless; on any AJAX‑driven or SPA‑style navigation it could lead to redundant listeners.

If you expect this script to live across navigations, consider also removing the resize listener there.

---

658-660: Ensure new line comments end with a period.

The new inline comment // change the opacity of the sticky menu to 1 does not end with a full stop. To match the coding guidelines you provided (every line comment should end with a period) and be consistent with other comments, please update it to:

-        // change the opacity of the sticky menu to 1
+        // Change the opacity of the sticky menu to 1.

---

600-625: Localise the screen-reader announcement string for WordPress localisation compliance.

The announce_section_change function at line 618 uses a hard-coded English string that should be wrapped with wp.i18n.__() and sprintf() to enable translation. WordPress JavaScript internationalisation best practices require using wp.i18n functions to wrap every user-visible string, e.g. sprintf( __( '%s items', 'my-text-domain' ), count ).

Update the announcement to:

const { __, sprintf } = wp.i18n;
announcer.textContent = sprintf( __( 'Navigated to %s section', 'tour-operator' ), section_title );

This approach mirrors your existing localisation pattern in index.js and ensures the script is enqueued with wp-i18n as a dependency so WordPress provides wp.i18n before your file runs. When using @wordpress/scripts, the import statement automatically declares the dependency for the built output.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro