Skip to content

feat(#3361): add JSDocs comments to wrappers and common types#3715

Open
bdfranck wants to merge 1 commit intodevfrom
benji/3159-add-jsdocs-to-wrappers
Open

feat(#3361): add JSDocs comments to wrappers and common types#3715
bdfranck wants to merge 1 commit intodevfrom
benji/3159-add-jsdocs-to-wrappers

Conversation

@bdfranck
Copy link
Copy Markdown
Collaborator

@bdfranck bdfranck commented Mar 31, 2026

React and Angular wrappers lacked JSDoc comments, giving consumers no inline documentation in their IDE when using components or inspecting props.

JSDoc added across all wrappers

  • Component-level descriptions on every exported class/function
  • Prop-level descriptions using consistent Sets the... phrasing, with @default, @required, and @deprecated tags where applicable
  • Common types (GoabInputOnChangeDetail, etc.) documented for IDE hover tooltips

Required/optional corrections (found during audit)

File Change
file-upload-card.ts size@Input({ required: true }), aligned with React + Svelte
file-upload-input.ts variant → optional with @default "dragdrop", aligned with React + Svelte
accordion.tsx heading → optional, aligned with Angular
popover.tsx target → required, aligned with Angular
drawer.ts, modal.ts, radio-item.ts Removed incorrect @required JSDoc tags from optional fields

Before (the change)

No inline documentation when consuming React/Angular wrapper components — no component descriptions, no prop hints, no type documentation visible in IDEs.

After (the change)

Hovering a component or prop in any IDE shows descriptions, defaults, deprecation notices, and required flags sourced directly from the wrapper source.

Make sure that you've checked the boxes below before you submit the PR

  • I have read and followed the setup steps
  • I have created necessary unit tests
  • I have tested the functionality in both React and Angular.

Steps needed to test

Open a project consuming @abgov/react-components or @abgov/angular-components and hover over any component or prop to verify JSDoc descriptions appear. Check @required, @default, and @deprecated tags render correctly in the IDE tooltip.

@bdfranck bdfranck linked an issue Mar 31, 2026 that may be closed by this pull request
@Spark450 Spark450 added the P1 Priority 1 (must have): Prevents product teams from adopting/using DS 2.0 as intended at launch. label Mar 31, 2026
@bdfranck bdfranck force-pushed the benji/3159-add-jsdocs-to-wrappers branch 3 times, most recently from ba51038 to 1813a47 Compare April 1, 2026 22:05
@bdfranck bdfranck requested a review from ArakTaiRoth April 1, 2026 22:15
@bdfranck bdfranck marked this pull request as ready for review April 1, 2026 22:16
@bdfranck bdfranck requested a review from vanessatran-ddi April 2, 2026 21:32
* // similar to app-footer-meta-section & app-footer-nav-section
*/
export class GoabFormItemSlot {
/** Sets which form-item slot this content should render into. */
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this have the @required in the comment?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added @required to the slot comment in b76d497: /** @required Sets which form-item slot this content should render into. */

@bdfranck
Copy link
Copy Markdown
Collaborator Author

bdfranck commented Apr 7, 2026

@copilot Make the following changes:

  • Change the comments from "Defines the..." with "Sets the..."
  • Change the testId comments to "Sets the data-testid attribute for automated testing."
  • Make the GoabFormItemSlot comment @required

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 7, 2026

@copilot Make the following changes:

  • Change the comments from "Defines the..." with "Sets the..."
  • Change the testId comments to "Sets the data-testid attribute for automated testing."
    ...

Done in b76d497:

  • Changed all "Defines the..." comments to "Sets the..." across Angular (badge.ts, input.ts, input-number.ts) and React (badge.tsx, modal.tsx, callout.tsx) wrappers
  • Updated testId comment in base.component.ts to "Sets the data-testid attribute for automated testing."
  • Added @required to the GoabFormItemSlot slot comment in form-item-slot.ts

Copy link
Copy Markdown
Collaborator

@vanessatran-ddi vanessatran-ddi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see some properties in angular documented as Sets the ... while some are not. I checked this rule https://github.com/GovAlta/ui-components/blob/dev/CLAUDE.md?plain=1#L109 and looks like we should follow the rule starts vs Sets the .. pattern. However it is not a blocker for me.

@ArakTaiRoth ArakTaiRoth removed their request for review April 7, 2026 21:36
@bdfranck
Copy link
Copy Markdown
Collaborator Author

bdfranck commented Apr 7, 2026

@copilot Address the above review comments.

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 7, 2026

@copilot Address the above review comments.

All review comments addressed in 97bf331:

  • badge.ts: Fixed content, iconType, ariaLabel to use "Sets the..."
  • block.ts: Fixed gap, direction, alignment to use "Sets the..."
  • button.ts: Fixed size, disabled, leadingIcon comments
  • accordion.tsx: Made heading optional, removed @required
  • drawer.ts: Removed incorrect @required from heading comment
  • file-upload-card.ts: Added required: true to size input + @required to JSDoc
  • file-upload-input.ts: Made variant optional (aligned with React/Svelte default "dragdrop")
  • modal.ts: Removed incorrect @required from heading comment
  • popover.tsx: Made target required, added @required
  • radio-item.ts: Removed incorrect @required from description comment

@bdfranck bdfranck requested a review from vanessatran-ddi April 7, 2026 22:30
@chrisolsen chrisolsen self-requested a review April 8, 2026 00:58
Co-Authored-By: bdfranck <1479091+bdfranck@users.noreply.github.com>
Co-Authored-By: Copilot <198982749+Copilot@users.noreply.github.com>
@bdfranck bdfranck force-pushed the benji/3159-add-jsdocs-to-wrappers branch from 97bf331 to d4fcb29 Compare April 8, 2026 02:13
@bdfranck bdfranck changed the title feat(#3159): add JSDocs to React and Angular component wrappers feat(#3361): add JSDocs comments to wrappers and common types Apr 8, 2026
Copy link
Copy Markdown
Collaborator

@vanessatran-ddi vanessatran-ddi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Comment on lines +57 to +58
constructor(private cdr: ChangeDetectorRef) {}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to remove this line.

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

Labels

P1 Priority 1 (must have): Prevents product teams from adopting/using DS 2.0 as intended at launch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JSDocs in Wrappers

5 participants