Skip to content

Conversation

@brianjhanson
Copy link
Contributor

@brianjhanson brianjhanson commented Dec 22, 2025

Refactors the settings/sites page into Inertia.

CleanShot.2026-01-06.at.14.57.55.mp4

@brianjhanson brianjhanson force-pushed the feature/inertia-settings-sites branch from 1bdd0eb to 64c5197 Compare December 22, 2025 21:54
@brianjhanson brianjhanson changed the base branch from feature/cms-1677-refactor-t-function to feature/inertia-settings-index January 6, 2026 21:04
@juice928
Copy link

👋 Hi, I'm an automated AI code review bot. I ran some checks on this PR and found 3 points that might be worth attention (could be false positives, please use your judgment):

  1. Improvement for ID generation to ensure better accessibility

    • Location: packages/craftcms-cp/src/components/nav-item/nav-item.ts:L105-L108
    • Impact: If an ID isn't provided, multiple components might end up with the same fallback ID, which can interfere with screen readers.
    • Suggestion: Consider adding a fallback mechanism to generate a unique ID so every element remains distinct and accessible.
  2. Enhancing slot detection to be more reactive

    • Location: packages/craftcms-cp/src/components/nav-item/nav-item.ts:L142-L146
    • Impact: Synchronous DOM checks might not catch dynamic content updates, which could lead to an inconsistent UI state.
    • Suggestion: You might find that using slotchange events or a observer helps the component react more smoothly to dynamic changes.
  3. Refining CSS selectors to maintain consistent styling

    • Location: packages/craftcms-cp/src/styles/shared/base.css:L43
    • Impact: Changing from a tag selector to a class-based one might cause standard code tags to lose their intended padding and background.
    • Suggestion: It might be safer to keep the global tag selector or verify that all instances of code elements have been updated to the new class.

If you find these suggestions disruptive, you can reply "stop" , and I'll automatically skip this repository in the future.

@brianjhanson brianjhanson marked this pull request as ready for review January 27, 2026 23:02
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.

3 participants