PR: Sticky Menu Mobile UX - First Section Open by Default#817
PR: Sticky Menu Mobile UX - First Section Open by Default#817
Conversation
- improve scroll's computing and behaviour
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
github-actions
bot
added
lang:php
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Warning
|
| Cohort / File(s) | Summary |
|---|---|
Source block component src/blocks/sticky-menu/index.js |
Added alignwide class to the nav element (Edit and Save render paths). |
Source block view logic src/blocks/sticky-menu/view.js |
Rewrote scroll target calc to use getBoundingClientRect + accumulated offsets; reworked active-section detection; adjusted initial scroll-spy and mobile section initialization to open first section by default and sync ARIA states. |
Source block styling src/blocks/sticky-menu/style.scss |
Added .lsx-to-sticky-menu-section-wrapper{margin:0} and changed .lsx-to-sticky-menu-list alignment from center to flex-start. |
Built view & assets build/blocks/sticky-menu/view.js, build/blocks/sticky-menu/view.asset.php, build/blocks/sticky-menu/index.asset.php, build/blocks/sticky-menu/index.js |
Compiled equivalents updated: scroll logic changes in view.js; PHP asset files’ version strings updated; minor formatting (trailing newline) in index.js. |
Built styling build/blocks/sticky-menu/style-index.css, build/blocks/sticky-menu/style-index-rtl.css |
Added wrapper margin reset and changed list alignment to flex-start to match source SCSS. |
Templates templates/single-accommodation.html, templates/single-country.html, templates/single-destination.html, templates/single-region.html, templates/single-tour.html |
Added is-layout-constrained to the sticky-menu wrapper and alignwide to the inner nav in each template (layout/alignment class changes only). |
Changelog changelog.md |
Added notes: mobile UX opens first section by default; tightened sticky-menu positioning and improved scroll-spy sync. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
- Pay attention to:
src/blocks/sticky-menu/view.js(scroll/top-offset math and mobile init),src/blocks/sticky-menu/style.scssand built CSS (layout alignment), templates where classes were added, and that build asset version changes correspond to source outputs.
Possibly related PRs
- feat: Add Sticky Menu Block with Desktop/Mobile Navigation #684 — modifies the same sticky-menu implementation and foundational behavior this PR refines.
- feat: Enhance Sticky Menu Block with Improved Inspector Controls, UX, and Keyboard Accessibility #773 — overlapping edits to scroll/active-section logic and styling in the sticky-menu block.
- feat: Complete Template Refactoring with Patterns, Template Parts, and WordPress Design Tokens #814 — changes to sticky-menu source/build files and template usage that intersect with this PR.
Suggested labels
comp:block-editor
Suggested reviewers
- krugazul
Poem
🐇 I hopped through code with nimble feet,
Margin reset, and items lined neat.
First section opens with a cheerful thump,
Scrolls align true — no sideways jump.
A sticky menu bloom, concise and sweet.
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately reflects the main change: enabling the first section to open by default on mobile in the sticky menu, which aligns with the primary objective and changelog entries. |
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
fix/sticky-menu-margins-styles
📜 Recent review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
build/blocks/sticky-menu/index.asset.php(1 hunks)build/blocks/sticky-menu/index.js(1 hunks)changelog.md(1 hunks)src/blocks/sticky-menu/index.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use semicolons to terminate statements in JavaScript block code
Ensure all generated code follows ESLint rules and WordPress Coding Standards before deployment
Files:
build/blocks/sticky-menu/index.jssrc/blocks/sticky-menu/index.js
**/*.{php,js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (GEMINI.md)
Follow WordPress Coding Standards including proper namespacing, security practices (sanitization/escaping), and code organization
Files:
build/blocks/sticky-menu/index.jssrc/blocks/sticky-menu/index.jsbuild/blocks/sticky-menu/index.asset.php
**/*.{js,jsx,ts,tsx,php}
📄 CodeRabbit inference engine (GEMINI.md)
Implement accessibility features following WCAG 2.2 AA standards in all block components and patterns
Files:
build/blocks/sticky-menu/index.jssrc/blocks/sticky-menu/index.jsbuild/blocks/sticky-menu/index.asset.php
**/*.{php,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (GEMINI.md)
Validate generated code against WordPress standards, test thoroughly, check security implications, and review for proper sanitization/escaping
Files:
build/blocks/sticky-menu/index.jssrc/blocks/sticky-menu/index.jsbuild/blocks/sticky-menu/index.asset.php
**/*.{js,jsx,ts,tsx,php,scss,css}
📄 CodeRabbit inference engine (GEMINI.md)
Use camelCase for variable and function names, kebab-case for CSS classes, and snake_case for PHP variable names per WordPress Coding Standards
Files:
build/blocks/sticky-menu/index.jssrc/blocks/sticky-menu/index.jsbuild/blocks/sticky-menu/index.asset.php
**/*.{js,jsx,ts,tsx,scss,css}
📄 CodeRabbit inference engine (GEMINI.md)
For performance optimization, implement code splitting, lazy loading, and minimize CSS/JavaScript bundle sizes in block components
Files:
build/blocks/sticky-menu/index.jssrc/blocks/sticky-menu/index.js
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,tsx,jsx}: Use async/await syntax; avoid .then() chains
Use named exports; avoid default exports unless wrapping a module
Files:
build/blocks/sticky-menu/index.jssrc/blocks/sticky-menu/index.js
src/blocks/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Block API v3 standards when developing blocks
Files:
src/blocks/sticky-menu/index.js
{src/blocks/**/*.{js,jsx,ts,tsx},src/**/*.php,inc/**/*.php}
📄 CodeRabbit inference engine (AGENTS.md)
Apply WCAG 2.2 AA accessibility standards to all blocks and UI components
Files:
src/blocks/sticky-menu/index.js
src/**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{js,ts,tsx,jsx}: Document public functions with JSDoc
Do not include any WordPress-related imports, functions, or file structures
Do not reference or assume plugins like WooCommerce or Wetu
Files:
src/blocks/sticky-menu/index.js
**/blocks/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
PHP render callbacks must be properly documented with descriptions of attributes and return values
Files:
build/blocks/sticky-menu/index.asset.php
**/*.php
📄 CodeRabbit inference engine (AGENTS.md)
Follow WordPress Coding Standards (WPCS) for all WordPress code
**/*.php: Store block patterns in the/patterns/directory at the root of the plugin (WordPress standard for plugins), not in/includes/patterns/
Use text domain 'tour-operator' for all localization strings in the plugin
Files:
build/blocks/sticky-menu/index.asset.php
**/*.{php,html}
📄 CodeRabbit inference engine (.github/instructions/patterns-templates-refactoring.instructions.md)
**/*.{php,html}: Always useLSX_TO_URLandLSX_TO_PATHconstants for asset paths in plugin files
Include CSS variable fallbacks with absolute hex values in inline styles for plugin compatibility with any theme
Use CSS variable formatvar(--wp--preset--[type]--[name], fallback_value)in inline styles, notvar:preset|[type]|[name]format
Usevar:preset|[type]|[name]format only within JSON block attributes, not in inline styles
Use the correct spacing preset names: 'tiny' (10px), 'small' (16px), 'medium' (32px), 'large' (48px), and 'x-small' (8px)
Files:
build/blocks/sticky-menu/index.asset.php
**/*.{php,twig}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not generate .php, .twig, or legacy CMS files
Files:
build/blocks/sticky-menu/index.asset.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (5)
build/blocks/sticky-menu/index.js (1)
1-1: Build artifact reflects source changes correctly.This minified build file has been regenerated to include the
alignwideclass additions from the source. Build artifacts should always be regenerated after source changes, which appears to have been done here.src/blocks/sticky-menu/index.js (2)
151-151: LGTM: Alignwide class correctly applied to navigation.The addition of the
alignwideclass to the navigation element in the Edit component correctly implements the layout improvements described in the PR objectives. This aligns with the broader template changes introducingis-layout-constrainedand wide alignment across the sticky menu system.
258-258: LGTM: Alignwide class consistently applied in Save component.The
alignwideclass is correctly applied to the navigation element in the Save component, maintaining consistency with the Edit component. This ensures the same layout behaviour on both the frontend and in the editor.build/blocks/sticky-menu/index.asset.php (1)
1-1: LGTM: Asset version correctly updated after source changes.The version hash has been properly updated to reflect the changes in the source JavaScript file. Dependencies remain unchanged as expected, since no new imports were added to the block.
changelog.md (1)
75-76: LGTM: Changelog entries accurately describe the sticky menu improvements.Both changelog entries correctly summarise the mobile UX and positioning enhancements described in the PR objectives. The entries are appropriately placed under "Block System Improvements" and provide clear, concise descriptions of the changes for end users.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
github-actions
bot
added
the
meta:needs-changelog
Summary of ChangesHello @tibiii, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on improving the mobile user experience of the sticky menu by ensuring the first section is open by default, making content immediately accessible. It also includes critical refinements to the scroll spy functionality and menu positioning logic, leading to more accurate and responsive behavior across various screen sizes and device types, while maintaining accessibility standards. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully improves the mobile user experience for the sticky menu by opening the first section by default. The changes also include significant improvements to the scroll-spy functionality by using getBoundingClientRect for more accurate section detection, which is a great enhancement. The code is well-structured, and the mobile-first section logic is implemented correctly. I've found one minor issue that should be addressed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/blocks/sticky-menu/index.js (1)
151-151: Inconsistency: Edit and Save components differ.The Edit component's navigation element at line 151 is missing the
alignwideclass that was added to the Save component at line 258. This creates a visual mismatch between the editor preview and the frontend display, potentially confusing content editors.🔎 Apply this diff to add the missing class:
- <nav className="lsx-to-sticky-menu-nav" aria-label={__('Page section navigation', 'tour-operator')}> + <nav className="lsx-to-sticky-menu-nav alignwide" aria-label={__('Page section navigation', 'tour-operator')}>
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
build/blocks/sticky-menu/index.asset.php(1 hunks)build/blocks/sticky-menu/index.js(1 hunks)build/blocks/sticky-menu/style-index-rtl.css(1 hunks)build/blocks/sticky-menu/style-index.css(1 hunks)build/blocks/sticky-menu/view.asset.php(1 hunks)build/blocks/sticky-menu/view.js(1 hunks)src/blocks/sticky-menu/index.js(1 hunks)src/blocks/sticky-menu/style.scss(2 hunks)src/blocks/sticky-menu/view.js(5 hunks)templates/single-accommodation.html(1 hunks)templates/single-country.html(1 hunks)templates/single-destination.html(1 hunks)templates/single-region.html(1 hunks)templates/single-tour.html(1 hunks)
🧰 Additional context used
📓 Path-based instructions (18)
**/*.{php,html}
📄 CodeRabbit inference engine (.github/instructions/patterns-templates-refactoring.instructions.md)
**/*.{php,html}: Always useLSX_TO_URLandLSX_TO_PATHconstants for asset paths in plugin files
Include CSS variable fallbacks with absolute hex values in inline styles for plugin compatibility with any theme
Use CSS variable formatvar(--wp--preset--[type]--[name], fallback_value)in inline styles, notvar:preset|[type]|[name]format
Usevar:preset|[type]|[name]format only within JSON block attributes, not in inline styles
Use the correct spacing preset names: 'tiny' (10px), 'small' (16px), 'medium' (32px), 'large' (48px), and 'x-small' (8px)
Files:
templates/single-destination.htmltemplates/single-accommodation.htmltemplates/single-region.htmlbuild/blocks/sticky-menu/view.asset.phpbuild/blocks/sticky-menu/index.asset.phptemplates/single-country.htmltemplates/single-tour.html
**/templates/*.html
📄 CodeRabbit inference engine (.github/instructions/patterns-templates-refactoring.instructions.md)
**/templates/*.html: Use<!-- wp:template-part {"slug":"[name]"} /-->syntax to include template parts in block templates
Use<!-- wp:pattern {"slug":"lsx-tour-operator/[pattern-name]"} /-->syntax to include patterns in block templates
Files:
templates/single-destination.htmltemplates/single-accommodation.htmltemplates/single-region.htmltemplates/single-country.htmltemplates/single-tour.html
**/{patterns,templates,parts}/*.{php,html}
📄 CodeRabbit inference engine (.github/instructions/patterns-templates-refactoring.instructions.md)
**/{patterns,templates,parts}/*.{php,html}: For icon blocks, use self-closing syntax and never embed HTML content inside icon block markup to prevent validation errors
Use primary color preset for buttons, borders, and main brand elements; use 'contrast' preset for text/links; use 'base' for white backgrounds
Files:
templates/single-destination.htmltemplates/single-accommodation.htmltemplates/single-region.htmltemplates/single-country.htmltemplates/single-tour.html
**/{templates,patterns,parts}/*.{php,html}
📄 CodeRabbit inference engine (.github/instructions/patterns-templates-refactoring.instructions.md)
Use wide alignment constraint for main content containers in single templates and align-full for full-width sections like breadcrumbs
Files:
templates/single-destination.htmltemplates/single-accommodation.htmltemplates/single-region.htmltemplates/single-country.htmltemplates/single-tour.html
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use semicolons to terminate statements in JavaScript block code
Ensure all generated code follows ESLint rules and WordPress Coding Standards before deployment
Files:
build/blocks/sticky-menu/index.jssrc/blocks/sticky-menu/index.jsbuild/blocks/sticky-menu/view.jssrc/blocks/sticky-menu/view.js
**/*.{php,js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (GEMINI.md)
Follow WordPress Coding Standards including proper namespacing, security practices (sanitization/escaping), and code organization
Files:
build/blocks/sticky-menu/index.jssrc/blocks/sticky-menu/index.jsbuild/blocks/sticky-menu/view.jsbuild/blocks/sticky-menu/view.asset.phpbuild/blocks/sticky-menu/index.asset.phpsrc/blocks/sticky-menu/view.js
**/*.{js,jsx,ts,tsx,php}
📄 CodeRabbit inference engine (GEMINI.md)
Implement accessibility features following WCAG 2.2 AA standards in all block components and patterns
Files:
build/blocks/sticky-menu/index.jssrc/blocks/sticky-menu/index.jsbuild/blocks/sticky-menu/view.jsbuild/blocks/sticky-menu/view.asset.phpbuild/blocks/sticky-menu/index.asset.phpsrc/blocks/sticky-menu/view.js
**/*.{php,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (GEMINI.md)
Validate generated code against WordPress standards, test thoroughly, check security implications, and review for proper sanitization/escaping
Files:
build/blocks/sticky-menu/index.jssrc/blocks/sticky-menu/index.jsbuild/blocks/sticky-menu/view.jsbuild/blocks/sticky-menu/view.asset.phpbuild/blocks/sticky-menu/index.asset.phpsrc/blocks/sticky-menu/view.js
**/*.{js,jsx,ts,tsx,php,scss,css}
📄 CodeRabbit inference engine (GEMINI.md)
Use camelCase for variable and function names, kebab-case for CSS classes, and snake_case for PHP variable names per WordPress Coding Standards
Files:
build/blocks/sticky-menu/index.jsbuild/blocks/sticky-menu/style-index.csssrc/blocks/sticky-menu/index.jsbuild/blocks/sticky-menu/view.jsbuild/blocks/sticky-menu/view.asset.phpbuild/blocks/sticky-menu/index.asset.phpsrc/blocks/sticky-menu/view.jssrc/blocks/sticky-menu/style.scssbuild/blocks/sticky-menu/style-index-rtl.css
**/*.{js,jsx,ts,tsx,scss,css}
📄 CodeRabbit inference engine (GEMINI.md)
For performance optimization, implement code splitting, lazy loading, and minimize CSS/JavaScript bundle sizes in block components
Files:
build/blocks/sticky-menu/index.jsbuild/blocks/sticky-menu/style-index.csssrc/blocks/sticky-menu/index.jsbuild/blocks/sticky-menu/view.jssrc/blocks/sticky-menu/view.jssrc/blocks/sticky-menu/style.scssbuild/blocks/sticky-menu/style-index-rtl.css
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,tsx,jsx}: Use async/await syntax; avoid .then() chains
Use named exports; avoid default exports unless wrapping a module
Files:
build/blocks/sticky-menu/index.jssrc/blocks/sticky-menu/index.jsbuild/blocks/sticky-menu/view.jssrc/blocks/sticky-menu/view.js
src/blocks/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Block API v3 standards when developing blocks
Files:
src/blocks/sticky-menu/index.jssrc/blocks/sticky-menu/view.js
{src/blocks/**/*.{js,jsx,ts,tsx},src/**/*.php,inc/**/*.php}
📄 CodeRabbit inference engine (AGENTS.md)
Apply WCAG 2.2 AA accessibility standards to all blocks and UI components
Files:
src/blocks/sticky-menu/index.jssrc/blocks/sticky-menu/view.js
src/**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{js,ts,tsx,jsx}: Document public functions with JSDoc
Do not include any WordPress-related imports, functions, or file structures
Do not reference or assume plugins like WooCommerce or Wetu
Files:
src/blocks/sticky-menu/index.jssrc/blocks/sticky-menu/view.js
**/blocks/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
PHP render callbacks must be properly documented with descriptions of attributes and return values
Files:
build/blocks/sticky-menu/view.asset.phpbuild/blocks/sticky-menu/index.asset.php
**/*.php
📄 CodeRabbit inference engine (AGENTS.md)
Follow WordPress Coding Standards (WPCS) for all WordPress code
**/*.php: Store block patterns in the/patterns/directory at the root of the plugin (WordPress standard for plugins), not in/includes/patterns/
Use text domain 'tour-operator' for all localization strings in the plugin
Files:
build/blocks/sticky-menu/view.asset.phpbuild/blocks/sticky-menu/index.asset.php
**/*.{php,twig}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not generate .php, .twig, or legacy CMS files
Files:
build/blocks/sticky-menu/view.asset.phpbuild/blocks/sticky-menu/index.asset.php
**/blocks/**/*.scss
📄 CodeRabbit inference engine (CLAUDE.md)
Sass/SCSS files for blocks must include both editor.scss and style.scss for editor and frontend styling respectively
Files:
src/blocks/sticky-menu/style.scss
🧬 Code graph analysis (3)
build/blocks/sticky-menu/index.js (2)
build/blocks/sticky-menu/view.js (43)
o(1-1)o(1-1)o(1-1)o(1-1)o(1-1)o(1-1)o(1-1)o(1-1)o(1-1)o(1-1)o(1-1)t(1-1)t(1-1)t(1-1)t(1-1)t(1-1)t(1-1)t(1-1)t(1-1)t(1-1)t(1-1)t(1-1)t(1-1)t(1-1)e(1-1)e(1-1)e(1-1)e(1-1)e(1-1)e(1-1)e(1-1)e(1-1)e(1-1)e(1-1)e(1-1)i(1-1)i(1-1)i(1-1)n(1-1)n(1-1)n(1-1)n(1-1)s(1-1)build/blocks/sticky-menu/sticky-menu-editor-extension.js (4)
t(1-1)r(1-1)a(1-1)l(1-1)
build/blocks/sticky-menu/view.js (1)
src/js/custom.js (1)
lsx_to(8-8)
src/blocks/sticky-menu/view.js (2)
build/blocks/sticky-menu/sticky-menu-editor-extension.js (1)
a(1-1)src/js/custom.js (1)
lsx_to(8-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (17)
build/blocks/sticky-menu/view.asset.php (1)
1-1: LGTM!Standard asset version hash update for the build artifact. This aligns with the changes made to the view script.
build/blocks/sticky-menu/view.js (1)
1-1: Verify source file changes.This is a minified build artifact. While the code appears to implement the scroll detection and mobile-first-section logic described in the PR, detailed review should be performed on the source file (src/blocks/sticky-menu/view.js) to ensure correctness of the scroll calculations, mobile section handling, and accessibility features.
build/blocks/sticky-menu/style-index-rtl.css (1)
1-1: LGTM!The RTL stylesheet correctly applies the wrapper margin reset and updates list alignment to
flex-start. These changes support the new constrained layout and align with the broader sticky menu layout improvements in this PR.templates/single-tour.html (1)
22-22: LGTM!The addition of
is-layout-constrainedto the sticky menu wrapper andalignwideto the navigation element correctly implements the layout improvements described in the PR objectives. This ensures the sticky menu respects the default section width constraints whilst providing appropriate wide alignment.templates/single-destination.html (1)
22-22: LGTM!Consistent with the changes in single-tour.html, the sticky menu correctly uses
is-layout-constrainedandalignwideclasses to implement the improved layout behaviour across all single destination templates.src/blocks/sticky-menu/style.scss (2)
1-3: LGTM!The new wrapper class with
margin: 0appropriately resets spacing to ensure proper layout control when used with theis-layout-constrainedclass applied in templates.
51-51: LGTM!Changing the menu list alignment from
centertoflex-startis more appropriate for the navigation menu items and aligns with the wider layout improvements in this PR. This provides a more standard left-aligned menu whilst still respecting the wide alignment constraint.build/blocks/sticky-menu/index.asset.php (1)
1-1: LGTM!Standard asset version hash update for the build artifact, consistent with the changes made to the block registration code.
templates/single-country.html (1)
22-22: LGTM! Layout classes correctly applied.The addition of
is-layout-constrainedon the wrapper andalignwideon the navigation element properly constrains the sticky menu to match the default section width, aligning with the PR objectives to improve positioning and visual consistency.templates/single-region.html (1)
22-22: LGTM! Consistent layout classes applied.The layout class additions match the changes across other templates, ensuring consistent sticky menu width and alignment behaviour throughout the site.
build/blocks/sticky-menu/style-index.css (1)
1-1: LGTM! CSS changes support layout improvements.The wrapper margin reset and left-alignment of menu items (via
justify-content: flex-start) properly support the constrained layout introduced in the templates, ensuring consistent styling and positioning.templates/single-accommodation.html (1)
22-22: LGTM! Consistent with other templates.Layout classes properly applied, maintaining consistency with the other single templates in this PR.
src/blocks/sticky-menu/index.js (1)
258-258: LGTM! Alignment class correctly added.The
alignwideclass on the navigation element properly implements the layout improvements described in the PR objectives.src/blocks/sticky-menu/view.js (4)
55-65: LGTM! Improved scroll position calculation.The refactored scroll calculation using
getBoundingClientRect()provides more accurate positioning than the previousoffsetTopapproach, especially for elements with CSS transforms or complex positioning. The offset accumulation logic correctly accounts for the admin bar, masthead, and sticky navigation.
289-306: LGTM! Enhanced active section detection.The refactored logic using
getBoundingClientRect()correctly identifies the active section by finding the one whose top edge is closest to (but not below) the offset line. The algorithm properly handles edge cases with the fallback to the first section when none are above the scroll position.
414-432: LGTM! Improved initial active section detection.The addition of visibility-based initial section detection with a 100ms delay ensures accurate identification of the active section on page load. The logic correctly filters visible sections within the viewport and selects the topmost one, with a sensible fallback to the first section.
533-555: LGTM! First section open by default on mobile.The index-based logic correctly implements the PR objective to keep the first section expanded on mobile whilst collapsing subsequent sections. The ARIA states (
aria-expanded) properly reflect the initial open state for screen reader accessibility, and the collapsed class is managed appropriately.
1 participant
name: "Pull Request"
about: "General changes, refactors, and maintenance"
title: "PR: Sticky Menu Mobile UX - First Section Open by Default"
labels: ["status:needs-review"]
Sticky Menu Mobile UX Improvements
Linked issues
Relates to sticky menu mobile usability
Changelog
Changed
Fixed
Risk Assessment
Risk Level: Low
Potential Impact:
Mitigation Steps:
aria-expanded) correctly reflect initial open stateHow to Test
Prerequisites
Test Steps
Mobile View Test:
Section Navigation:
Desktop Verification:
Expected Results
Mobile (<768px):
aria-expanded="true"aria-expanded="false"Desktop (≥768px):
Edge Cases to Verify
Checklist (Global DoD / PR)
References
Summary by CodeRabbit
Bug Fixes
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.