-
-
Notifications
You must be signed in to change notification settings - Fork 0
Fix: Sticky Menu Layout Alignment and Global Padding #819
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
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Warning Rate limit exceeded@tibiii has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 41 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
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 |
|---|---|
Block Component src/blocks/sticky-menu/index.js |
Updated CSS class names in JSX: replaced nav class from "lsx-to-sticky-menu-nav alignwide" to "lsx-to-sticky-menu-nav is-layout-constrained"; replaced ul class from "lsx-to-sticky-menu-list" to "lsx-to-sticky-menu-list alignwide". Changes applied to both Edit and Save components. |
Template Files templates/single-accommodation.html, templates/single-country.html, templates/single-destination.html, templates/single-region.html, templates/single-tour.html |
Refactored sticky-menu block markup across all templates following a consistent pattern: removed is-layout-constrained from outer wrapper divs, added is-layout-constrained and has-global-padding to inner nav elements, added alignwide class to ul list containers. Layout scaffolding and padding behaviour adjusted whilst preserving menu items and data attributes. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
- Review the class mapping logic in
src/blocks/sticky-menu/index.jsto confirm the correct class substitutions are applied in both Edit and Save components - Verify that the consistent pattern of class migrations across the five template files matches the block component changes and maintains visual/layout consistency
Possibly related PRs
- PR: Sticky Menu Mobile UX - First Section Open by Default #817 — Modifies the same sticky-menu markup and class usage across nav/ul wrapper elements in block and template files
- feat: Add Sticky Menu Block with Desktop/Mobile Navigation #684 — Introduced the sticky-menu block structure that this PR's edits directly refactor
- feat: Enhance Sticky Menu Block with Improved Inspector Controls, UX, and Keyboard Accessibility #773 — Touches sticky-menu block markup and class name updates aligned with this PR's layout restructuring
Suggested labels
comp:block-editor, area:block-editor, lang:javascript, lang:css, status:needs-review
Suggested reviewers
- krugazul
Poem
🐰 A sticky menu springs to life,
With classes shuffled left and right,
Constrained to nav, aligned just so,
Padded globally, let layouts flow! ✨
Pre-merge checks and finishing touches
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately summarizes the main changes: restructuring CSS classes for the sticky menu's layout alignment and global padding across the component implementation and templates. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
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.
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 resolves layout alignment inconsistencies within the sticky menu component across various single post type templates. The changes involve a strategic restructuring of CSS classes, moving 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
|
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.
Code Review
This pull request aims to fix layout alignment issues for the sticky menu by restructuring CSS classes to better align with WordPress block editor standards. The changes correctly move width-constraining classes to inner elements for more granular control, and these changes are applied to both the block's Edit and Save components, as well as updated in the HTML templates. However, I've identified an inconsistency: the updated HTML templates add the has-global-padding class to the <nav> element, but this class is missing from the block's Edit and Save functions in src/blocks/sticky-menu/index.js. This will cause new or updated instances of the block to render without the intended padding. I've added comments with suggestions to resolve this. Overall, the approach is sound, but this oversight should be addressed.
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: 2
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
src/blocks/sticky-menu/index.js(2 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 (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:
src/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:
src/blocks/sticky-menu/index.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:
src/blocks/sticky-menu/index.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:
src/blocks/sticky-menu/index.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:
src/blocks/sticky-menu/index.js
**/*.{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:
src/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
**/*.{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:
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
**/*.{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-region.htmltemplates/single-country.htmltemplates/single-accommodation.htmltemplates/single-destination.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-region.htmltemplates/single-country.htmltemplates/single-accommodation.htmltemplates/single-destination.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-region.htmltemplates/single-country.htmltemplates/single-accommodation.htmltemplates/single-destination.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-region.htmltemplates/single-country.htmltemplates/single-accommodation.htmltemplates/single-destination.htmltemplates/single-tour.html
⏰ 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 (7)
src/blocks/sticky-menu/index.js (2)
154-154: LGTM: Alignment class correctly moved to list element.The
alignwideclass has been correctly applied to the list element, which aligns with the WordPress block layout pattern where the constrained container's inner content can be wide-aligned.
260-260: LGTM: Save component list alignment updated correctly.The
alignwideclass has been correctly applied to the list element in the Save component, maintaining consistency with the Edit component.templates/single-country.html (1)
22-22: LGTM: Template markup correctly updated with new layout classes.The sticky menu markup has been correctly updated with
is-layout-constrained has-global-paddingon the nav element andalignwideon the list element. This follows WordPress block layout conventions and maintains all accessibility attributes.templates/single-region.html (1)
22-22: LGTM: Consistent layout class updates applied.The template has been correctly updated with the new layout class structure, consistent with the other templates in this PR.
templates/single-tour.html (1)
22-22: LGTM: Layout classes correctly applied to tour template.The sticky menu markup follows the same correct pattern as the other templates, with proper layout classes and preserved accessibility attributes.
templates/single-destination.html (1)
22-22: LGTM: Destination template updated correctly.The layout class refactoring has been applied correctly, maintaining consistency with the other single templates.
templates/single-accommodation.html (1)
22-22: LGTM: Accommodation template correctly updated.The sticky menu markup has been correctly updated with the new layout class structure, consistent with all other templates in this PR.
Description
This PR fixes the sticky menu layout alignment by restructuring the CSS class hierarchy to properly utilize WordPress block layout utilities and global padding.
Changes
Block Implementation (
src/blocks/sticky-menu/index.js)alignwidetois-layout-constrainedto enable proper container behavioralignwideclass from nav to ul for correct content width constraintEditandSavecomponents for consistencyTemplate Updates
Updated all single post type templates to reflect the new class structure:
templates/single-accommodation.htmltemplates/single-country.htmltemplates/single-destination.htmltemplates/single-region.htmltemplates/single-tour.htmlEach template now includes:
is-layout-constrained has-global-paddingon the nav elementalignwideon the list elementWhy This Change?
The previous implementation applied
alignwidedirectly to the nav element, which prevented proper use of WordPress's layout system. By restructuring:is-layout-constrained: Establishes the nav as a constrained layout containeralignwide: Constrains the menu items to the wide content area while maintaining proper paddingThis approach aligns with WordPress block editor best practices and ensures consistent spacing with other blocks in the template.
Testing
Related Issues
Fixes alignment inconsistencies in the sticky menu component across all single post type templates.
Checklist
npm run build)Type: Bug Fix
Priority: Normal
Area: Block Editor, Templates
Components: Sticky Menu Block, Block Templates
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.