Skip to content

ECHOES-1410 Add id props to main layout slots#714

Merged
gregaubert merged 1 commit into
mainfrom
greg/id-to-layout-slots
Jul 2, 2026
Merged

ECHOES-1410 Add id props to main layout slots#714
gregaubert merged 1 commit into
mainfrom
greg/id-to-layout-slots

Conversation

@gregaubert

@gregaubert gregaubert commented Jul 2, 2026

Copy link
Copy Markdown
Member

Summary by Gitar

  • Component Enhancements:
    • Added optional id prop to PageGridProps interface in LayoutSlots.tsx.
    • Added optional id prop to PageContentProps interface in LayoutSlots.tsx.

This will update automatically on new commits.

@netlify

netlify Bot commented Jul 2, 2026

Copy link
Copy Markdown

Deploy Preview for echoes-react ready!

Name Link
🔨 Latest commit 301e49e
🔍 Latest deploy log https://app.netlify.com/projects/echoes-react/deploys/6a4624b2834d350008bf3c14
😎 Deploy Preview https://deploy-preview-714--echoes-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod Bot changed the title Add id props to main layout slots ECHOES-1410 Add id props to main layout slots Jul 2, 2026
@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Jul 2, 2026

Copy link
Copy Markdown

ECHOES-1410

Comment thread src/components/layout/LayoutSlots.tsx
@gitar-bot

gitar-bot Bot commented Jul 2, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 1 resolved / 1 findings

Adds optional id props to PageGrid and PageContent components to improve layout accessibility. No issues found.

✅ 1 resolved
Quality: New id public props lack required TSDoc comments

📄 src/components/layout/LayoutSlots.tsx:106 📄 src/components/layout/LayoutSlots.tsx:206
The custom review conventions require a /** */ doc comment on every field of a public props interface. The newly added id?: string; props on PageGridProps (LayoutSlots.tsx:106) and PageContentProps (LayoutSlots.tsx:206) are undocumented. Add a TSDoc comment to each, e.g. describing that the id is applied to the underlying container element.

Note: the props are correctly forwarded to the underlying elements via ...restProps in both PageGrid and PageContent, so functionality is fine — this is purely a documentation-convention gap. (The pre-existing className and AsideProps.id are outside this diff.)

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@gitar-bot

gitar-bot Bot commented Jul 2, 2026

Copy link
Copy Markdown

Understood — id is certainly intuitive, but the project convention (defined in .gitar/review/naming-and-api.md) requires a TSDoc comment on every field of a public props interface, regardless of how self-explanatory it is. Consistent documentation makes IDE hover-docs and auto-generated API references complete without exceptions.

That said, if the team wants to relax this rule for widely-known HTML-attribute props like id, className, etc., that would be a reasonable policy change to make in the .gitar/review/naming-and-api.md convention file. For now I'll flag it as-is per the current rule.

@sonarqube-next

sonarqube-next Bot commented Jul 2, 2026

Copy link
Copy Markdown

@gregaubert gregaubert merged commit 3f0bcfd into main Jul 2, 2026
15 checks passed
@gregaubert gregaubert deleted the greg/id-to-layout-slots branch July 2, 2026 08:59
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