Skip to content

Conversation

@d0ubIeU
Copy link
Contributor

@d0ubIeU d0ubIeU commented Dec 11, 2025

Updated index.twig to hide the simple search bar when already on a search results page. This prevents redundant search input options and streamlines the user experience, while keeping the advanced search functionality available.

Summary by CodeRabbit

  • Style

    • Enhanced dark/high-contrast theme: bolder search input and badges, stronger borders, adjusted link and heading sizing/spacing, and improved contrast across cards, footer, alerts, and content links.
  • Bug Fixes

    • Search block suppressed on dedicated search pages and when the site is in maintenance mode.

✏️ Tip: You can customize this high-level summary in your review settings.

Replaces inline styles for required field asterisks in add.twig and ask.twig templates with a reusable .pmf-required-asterisk CSS class. Adds corresponding styles to _global.scss and _theme-switcher.scss for consistent appearance across themes.
Replaced hardcoded color values in the high contrast theme with Bootstrap CSS variables for better maintainability and consistency. Enhanced accessibility and visual appearance of links, logo, headings, and form elements. Removed Atkinson Hyperlegible font files from the assets.
Scaled up the #phpmyfaq-logo for better visibility and applied color inversion to the logo image for high-contrast mode. Simplified logo image styling and removed redundant hover effects.
Moves the required asterisk inside the conditional so it only appears when the category field is required, improving form label accuracy.
Increased font weight in badge elements for better readability, and adjusted badge border color. Updated index.twig to hide the simple search bar when already on a search results page. This prevents redundant search input options and streamlines the user experience, while keeping the advanced search functionality available.
@coderabbitai
Copy link

coderabbitai bot commented Dec 11, 2025

Walkthrough

High-contrast SCSS adjustments (weights, borders, color/border tweaks) and a template change that detects search pages to skip rendering the search block on those pages.

Changes

Cohort / File(s) Summary
High-contrast theme styling
phpmyfaq/assets/scss/layout/_theme-switcher.scss
Multiple high-contrast/dark-mode CSS changes: set search input font-weight to 800; increase badge border from 3px to 4px and add badge font-weight 800; adjust global link/heading sizing and spacing; update various colors/borders for cards, footer, alerts, badges, and related spacing/background-clip tweaks.
Search page detection logic
phpmyfaq/assets/templates/default/index.twig
Add local isSearchPage variable (checks for /search.html or action=search) and change the SEARCH block to not render when in maintenance mode or when isSearchPage is true.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Inspect CSS specificity and cascade impacts in _theme-switcher.scss, especially for high-contrast overrides and heading/background-clip spacing.
  • Verify isSearchPage pattern covers all search routes and edge cases in index.twig, including query parameter variations and localized paths.

Possibly related PRs

  • thorsten/phpMyFAQ#3797 — Overlapping edits to _theme-switcher.scss touching high-contrast/dark-mode styles (badges, forms).

Suggested labels

Improvement

Suggested reviewers

  • thorsten

Poem

🐇 I nudged the theme to bolden night and day,

Thickened badges, text that loudly says hooray.
On search pages I pause my little hop,
So results can shine — I sit and watch, then stop. 🥕

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 describes the main change: hiding the simple search bar on search results pages in index.twig, which aligns with the primary modifications shown in the file summary.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6302a1d and 03b3853.

📒 Files selected for processing (2)
  • phpmyfaq/assets/scss/layout/_theme-switcher.scss (5 hunks)
  • phpmyfaq/assets/templates/default/index.twig (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{html,css,scss}

📄 CodeRabbit inference engine (AGENTS.md)

Use HTML5 and CSS3 standards for frontend code

Files:

  • phpmyfaq/assets/scss/layout/_theme-switcher.scss
**/*.{html,scss,css}

📄 CodeRabbit inference engine (AGENTS.md)

Use Bootstrap components and utilities for layout and styling

Files:

  • phpmyfaq/assets/scss/layout/_theme-switcher.scss
🧠 Learnings (1)
📚 Learning: 2025-11-29T10:03:37.303Z
Learnt from: CR
Repo: thorsten/phpMyFAQ PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-29T10:03:37.303Z
Learning: Follow accessibility best practices to ensure the application is usable by all users

Applied to files:

  • phpmyfaq/assets/scss/layout/_theme-switcher.scss
⏰ 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). (4)
  • GitHub Check: phpMyFAQ 8.6 Test on ubuntu-latest
  • GitHub Check: phpMyFAQ 8.5 Test on ubuntu-latest
  • GitHub Check: phpMyFAQ 8.3 Test on ubuntu-latest
  • GitHub Check: phpMyFAQ 8.4 Test on ubuntu-latest
🔇 Additional comments (6)
phpmyfaq/assets/templates/default/index.twig (1)

103-104: Excellent fix for URL matching precision!

The regex patterns correctly address the previous concern about false positives. The boundaries ([?#]|$) and (&|$) ensure that:

  • /search.html only matches when followed by query params, hash, or end of URL
  • action=search only matches as a complete query parameter

This prevents unintended matches like "research.html" or "action=search_settings" while correctly hiding the search bar on search results pages.

phpmyfaq/assets/scss/layout/_theme-switcher.scss (5)

303-303: Good accessibility enhancement for search input.

Adding bold font-weight to the search input in high-contrast mode improves readability for users who need enhanced visual contrast.

Based on learnings, this follows accessibility best practices.


365-371: Perfect implementation of scoped link styling!

This change directly addresses the previous review concern about universal link styling causing layout issues. By applying font-size: 1.3rem and font-weight: 800 only to .pmf-content a, main a, the code:

  • Preserves enhanced readability for primary content links
  • Prevents layout breakage in navigation, tables, and inline contexts
  • Maintains UI component integrity while supporting accessibility needs

Based on learnings, this follows accessibility best practices.


395-396: LGTM - minor formatting improvement.

The spacing adjustment improves readability without changing functionality.


470-471: Enhanced badge visibility for high-contrast mode.

Increasing the border thickness to 4px and adding bold font-weight improves badge visibility and readability in high-contrast mode, which is particularly important for these small UI elements.

Based on learnings, this follows accessibility best practices.


571-571: LGTM - color format simplification.

Using #000 instead of #000000 is functionally equivalent and follows modern CSS conventions for shorter hex codes.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (2)
phpmyfaq/assets/scss/layout/_theme-switcher.scss (2)

464-465: Review badge border thickness for potential layout issues.

The border: 4px solid var(--bs-dark) on badges in high-contrast mode adds 8px total width/height to each badge. This could cause layout shifts or text wrapping in tight spaces.

Consider using a thinner border or outline:

  .badge {
-   border: 4px solid var(--bs-dark);
+   border: 2px solid var(--bs-dark);
    font-weight: 800;

Or use outline to avoid affecting layout dimensions:

  .badge {
-   border: 4px solid var(--bs-dark);
+   outline: 3px solid var(--bs-dark);
+   outline-offset: -3px;
    font-weight: 800;

572-577: Performance consideration: Universal transition selector.

The universal selector * applying transitions to all elements can impact rendering performance, especially on pages with many DOM nodes.

Consider scoping transitions to specific elements:

-* {
+body,
+.card,
+.btn,
+.form-control,
+.alert,
+a,
+.navbar,
+.dropdown-menu {
  transition:
    background-color 0.3s ease,
    border-color 0.3s ease,
    color 0.3s ease;
}

This reduces the number of elements being transitioned and improves performance without sacrificing the smooth theme-switching experience on key UI elements.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0203e64 and 4bac003.

⛔ Files ignored due to path filters (4)
  • phpmyfaq/assets/fonts/AtkinsonHyperlegible-Bold.ttf is excluded by !**/*.ttf
  • phpmyfaq/assets/fonts/AtkinsonHyperlegible-BoldItalic.ttf is excluded by !**/*.ttf
  • phpmyfaq/assets/fonts/AtkinsonHyperlegible-Italic.ttf is excluded by !**/*.ttf
  • phpmyfaq/assets/fonts/AtkinsonHyperlegible-Regular.ttf is excluded by !**/*.ttf
📒 Files selected for processing (5)
  • phpmyfaq/assets/scss/_global.scss (1 hunks)
  • phpmyfaq/assets/scss/layout/_theme-switcher.scss (7 hunks)
  • phpmyfaq/assets/templates/default/add.twig (6 hunks)
  • phpmyfaq/assets/templates/default/ask.twig (4 hunks)
  • phpmyfaq/assets/templates/default/index.twig (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{html,css,scss}

📄 CodeRabbit inference engine (AGENTS.md)

Use HTML5 and CSS3 standards for frontend code

Files:

  • phpmyfaq/assets/scss/_global.scss
  • phpmyfaq/assets/scss/layout/_theme-switcher.scss
**/*.{html,scss,css}

📄 CodeRabbit inference engine (AGENTS.md)

Use Bootstrap components and utilities for layout and styling

Files:

  • phpmyfaq/assets/scss/_global.scss
  • phpmyfaq/assets/scss/layout/_theme-switcher.scss
🔇 Additional comments (8)
phpmyfaq/assets/scss/_global.scss (1)

72-76: LGTM! Clean centralization of required-field styling.

The new .pmf-required-asterisk class effectively consolidates required-field indicator styling across templates, using Bootstrap's CSS variable for consistent theming.

phpmyfaq/assets/templates/default/add.twig (1)

31-31: LGTM! Consistent refactoring to class-based styling.

The replacement of inline styling with the .pmf-required-asterisk class is applied consistently across all form fields, improving maintainability and theme compatibility.

Also applies to: 42-42, 53-53, 71-71, 84-84, 95-95

phpmyfaq/assets/templates/default/index.twig (1)

104-124: LGTM! Proper conditional rendering.

The updated condition correctly hides the search block when on a search page, preventing redundant search UI while maintaining accessibility to advanced search via the link.

phpmyfaq/assets/templates/default/ask.twig (1)

35-35: LGTM! Consistent required-field markup.

The explicit <span class="pmf-required-asterisk"> *</span> markup is applied consistently, with appropriate conditional rendering for id5 where the field requirement may vary.

Also applies to: 44-44, 54-54, 73-73

phpmyfaq/assets/scss/layout/_theme-switcher.scss (4)

144-147: LGTM! Dark mode support for required indicators.

The .pmf-required-asterisk styling correctly applies the danger color variable in dark mode for consistency.


258-260: LGTM! Maximum contrast background.

The high-contrast mode correctly uses CSS variables for consistent dark background and light text.


410-414: LGTM! High-contrast styling for required indicators.

The .pmf-required-asterisk in high-contrast mode appropriately uses the primary color with bold weight for maximum visibility.


367-385: No changes needed. The logo styling in high-contrast mode is part of an intentional accessibility feature with proper theme switching and testing infrastructure in place. The inverted filter, 1.3x scale, and yellow border are deliberate design choices for this mode and do not require verification—they function as designed.

regex URL matching to avoid false positives for search pages in index.twig
Apply larger sizing <a> only to main content links to preserving layout integrity.
@thorsten thorsten merged commit 4b2671b into thorsten:main Dec 12, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants