Keep nav menu + Ask AI button in single policy view (#24)#32
Merged
Conversation
📝 WalkthroughWalkthroughThis PR extracts the trust center navigation into ChangesNavigation Partial Extraction and Template Refactoring
Sequence Diagram(s)sequenceDiagram
participant TrustCenter as templates/trust-center.php
participant PolicySingle as templates/partials/policy-single.php
participant Nav as templates/partials/nav.php
TrustCenter->>TrustCenter: build $ot_nav_items
TrustCenter->>Nav: include (ot_nav_scrollspy = true)
PolicySingle->>PolicySingle: set ot_nav_anchor_base, ot_nav_scrollspy = false
PolicySingle->>Nav: include shared nav partial
Nav->>Nav: render brand, nav items, Ask AI CTA
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
4 tasks
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@languages/open-trust-center-by-ettic-nl_NL.po`:
- Around line 2388-2394: Provide Dutch translations for the two empty msgstr
entries by filling the msgstr for msgid "Skip to content" (use a proper Dutch
phrase like "Sla naar inhoud over" or "Ga naar inhoud") and for msgid "Trust
center navigation" (use "Navigatie vertrouwenscentrum" or similar), ensuring the
translations replace the empty strings directly in the PO entries so nl_NL users
see localized ARIA/navigation labels; update the entries where msgid equals
"Skip to content" and "Trust center navigation".
In `@templates/partials/nav.php`:
- Around line 25-30: The brand image can end up with an empty alt when
$ot_company_name is empty, breaking the accessible name for the brand link;
update the rendering logic around $ot_logo_url and $ot_company_name so the image
alt and/or the surrounding brand link always use a non-empty accessible name
(fall back to get_bloginfo('name') when $ot_company_name is empty).
Specifically, ensure the img alt uses esc_attr($ot_company_name ?:
get_bloginfo('name')) and/or add an aria-label on the brand link that uses the
same fallback so the link always has an accessible name (refer to variables
$ot_logo_url, $ot_company_name and the element with class
ettic-otc-nav__brand-logo / ettic-otc-nav__brand-name).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f00a2484-53da-48be-8bdb-d933ccb7eb31
📒 Files selected for processing (5)
languages/open-trust-center-by-ettic-nl_NL.polanguages/open-trust-center-by-ettic.pottemplates/partials/nav.phptemplates/partials/policy-single.phptemplates/trust-center.php
Comment on lines
+2388
to
2394
| #: templates/chat.php:100 templates/trust-center.php:111 | ||
| msgid "Skip to content" | ||
| msgstr "" | ||
|
|
||
| #: templates/chat.php:102 templates/partials/policy-single.php:36 | ||
| #: templates/trust-center.php:114 | ||
| #: templates/chat.php:102 templates/partials/nav.php:22 | ||
| msgid "Trust center navigation" | ||
| msgstr "" |
There was a problem hiding this comment.
Translate newly added navigation accessibility labels in Dutch.
The new entries for navigation accessibility text are still empty, so nl_NL users will see English strings for these UI/ARIA labels.
Proposed fix
#: templates/chat.php:100 templates/trust-center.php:111
msgid "Skip to content"
-msgstr ""
+msgstr "Ga naar inhoud"
#: templates/chat.php:102 templates/partials/nav.php:22
msgid "Trust center navigation"
-msgstr ""
+msgstr "Trust center-navigatie"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #: templates/chat.php:100 templates/trust-center.php:111 | |
| msgid "Skip to content" | |
| msgstr "" | |
| #: templates/chat.php:102 templates/partials/policy-single.php:36 | |
| #: templates/trust-center.php:114 | |
| #: templates/chat.php:102 templates/partials/nav.php:22 | |
| msgid "Trust center navigation" | |
| msgstr "" | |
| #: templates/chat.php:100 templates/trust-center.php:111 | |
| msgid "Skip to content" | |
| msgstr "Ga naar inhoud" | |
| #: templates/chat.php:102 templates/partials/nav.php:22 | |
| msgid "Trust center navigation" | |
| msgstr "Trust center-navigatie" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@languages/open-trust-center-by-ettic-nl_NL.po` around lines 2388 - 2394,
Provide Dutch translations for the two empty msgstr entries by filling the
msgstr for msgid "Skip to content" (use a proper Dutch phrase like "Sla naar
inhoud over" or "Ga naar inhoud") and for msgid "Trust center navigation" (use
"Navigatie vertrouwenscentrum" or similar), ensuring the translations replace
the empty strings directly in the PO entries so nl_NL users see localized
ARIA/navigation labels; update the entries where msgid equals "Skip to content"
and "Trust center navigation".
Fall back to site name for the logo alt so the brand link always has an accessible label. Addresses CodeRabbit a11y note.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
languages/open-trust-center-by-ettic-nl_NL.po (1)
2388-2394:⚠️ Potential issue | 🟡 MinorTranslate newly added navigation accessibility labels in Dutch.
The new entries for navigation accessibility text are still empty, so nl_NL users will see English strings for these UI/ARIA labels.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@languages/open-trust-center-by-ettic-nl_NL.po` around lines 2388 - 2394, Add Dutch translations for the two empty msgstr entries by replacing the empty msgstr values for the msgid "Skip to content" and "Trust center navigation" with appropriate Dutch strings (e.g., "Sla naar inhoud over" or "Ga naar inhoud" for "Skip to content" and "Navigatie vertrouwenscentrum" for "Trust center navigation") so nl_NL users see localized ARIA/navigation labels; update the msgstr for those exact msgid entries in the PO file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@languages/open-trust-center-by-ettic-nl_NL.po`:
- Around line 2388-2394: Add Dutch translations for the two empty msgstr entries
by replacing the empty msgstr values for the msgid "Skip to content" and "Trust
center navigation" with appropriate Dutch strings (e.g., "Sla naar inhoud over"
or "Ga naar inhoud" for "Skip to content" and "Navigatie vertrouwenscentrum" for
"Trust center navigation") so nl_NL users see localized ARIA/navigation labels;
update the msgstr for those exact msgid entries in the PO file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 89273217-92d6-47dd-a864-8afbde21e51f
📒 Files selected for processing (2)
languages/open-trust-center-by-ettic-nl_NL.polanguages/open-trust-center-by-ettic.pot
✅ Files skipped from review due to trivial changes (1)
- languages/open-trust-center-by-ettic.pot
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #26, which was branched pre-rename and edits the deleted
opentrust.potwith oldot-/data-ot-nav/OPENTRUST_PLUGIN_DIRidentifiers, so it can no longer merge. This re-applies the same fix onto the current templates.Problem (#24)
The single policy view rendered a brand-only nav bar — the section menu (Policies, Certifications, …) and the Ask AI CTA were missing, so visitors lost navigation the moment they opened a policy.
Change
templates/partials/nav.php, used by both the main page and the single policy view.trust-center.phpbefore the view branch and shared across both views.#ettic-otc-*anchors, scroll-spy on — no behavior change.{base}#ettic-otc-*); scroll-spy off since those sections don't exist there.Test plan
Closes #24
🤖 Generated with Claude Code
Summary by CodeRabbit
Internationalization
Refactor
New Features