Skip to content

Conversation

richardgaunt
Copy link
Collaborator

@richardgaunt richardgaunt commented Sep 18, 2025

JIRA ticket: #3547385

Checklist before requesting a review

  • I have formatted the subject to include ticket number as
  • I have added a link to the issue tracker
  • I have provided information in section about WHY something was done if this was not a normal implementation
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • I have run new and existing relevant tests locally with my changes, and they passed
  • I have provided screenshots, where applicable

Changed

  1. Removed component namespaces.

Screenshots

Summary by CodeRabbit

  • Refactor

    • Removed deprecated component namespace configuration from theme settings.
    • Page layout now uses a consolidated theme template, which may alter which template is rendered for pages.
  • Chores

    • Cleaned up redundant declarations and outdated comments in theme configuration files.
  • Documentation

    • Minor comment tidy-ups to improve clarity within theme configuration notes.

@richardgaunt richardgaunt added the State: Needs review Pull requests needs a review from assigned developers label Sep 18, 2025
Copy link

coderabbitai bot commented Sep 18, 2025

Walkthrough

Removed component namespace declarations from two CivicTheme .info.yml files and changed the page layout include from @templates/page/page.twig to civictheme:page. No other functional changes.

Changes

Cohort / File(s) Summary
Info.yml removals
web/themes/contrib/civictheme/civictheme.info.yml, web/themes/contrib/civictheme/civictheme_starter_kit/civictheme_starter_kit.info.yml
Removed the componentsnamespaces block that mapped base, atoms, molecules, organisms, templates (and ct-* variants). Starter kit also had a comment removed under ckeditor5-stylesheets.
Template include change
web/themes/contrib/civictheme/templates/layout/page.html.twig
Replaced include from {% include '@templates/page/page.twig' %} to {% include 'civictheme:page' %}.

Sequence Diagram(s)

sequenceDiagram
    participant Renderer as Theme renderer
    participant Old as @templates/page/page.twig
    participant New as civictheme:page

    rect rgb(230,240,255)
    Note right of Renderer: Previous flow
    Renderer->>Old: include '@templates/page/page.twig'
    Old-->>Renderer: render page template
    end

    rect rgb(230,255,230)
    Note right of Renderer: New flow
    Renderer->>New: include 'civictheme:page'
    New-->>Renderer: render page template
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • AlexSkrypnyk
  • alan-cole

Poem

I hopped through files both near and far,
Nudged namespaces out from under par.
A new include, a tidy trail—
My whiskers twitch, I leave a trail.
Carrots ready, config neat, hooray! 🥕

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 clearly describes the primary change of removing component namespaces and references the relevant ticket, making it concise, specific, and accurately reflecting the changeset.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/3547385-remove-component-namespaces

📜 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 5551037 and b308761.

📒 Files selected for processing (1)
  • web/themes/contrib/civictheme/templates/layout/page.html.twig (1 hunks)
⏰ 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). (2)
  • GitHub Check: build-and-push
  • GitHub Check: commit
🔇 Additional comments (1)
web/themes/contrib/civictheme/templates/layout/page.html.twig (1)

12-12: Verify removal of old component namespaces in .info.yml
The new civictheme:page include resolves correctly, and no @templates references remain. However, web/themes/contrib/civictheme/civictheme.info.yml still lists a components:components library. Confirm all old component namespaces and related libraries (components, atoms, molecules, etc.) have been removed from your theme’s .info.yml.


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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

State: Needs review Pull requests needs a review from assigned developers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants