Skip to content

docs(FR-2897): clarify modal footer prop-first guidance in react-modal-drawer skill#7420

Merged
yomybaby merged 1 commit into
mainfrom
docs/FR-2897-modal-footer-props-default
May 14, 2026
Merged

docs(FR-2897): clarify modal footer prop-first guidance in react-modal-drawer skill#7420
yomybaby merged 1 commit into
mainfrom
docs/FR-2897-modal-footer-props-default

Conversation

@yomybaby
Copy link
Copy Markdown
Member

@yomybaby yomybaby commented May 14, 2026

Resolves #7419 (FR-2897)

Summary

  • Rewrote Section 6 of the react-modal-drawer skill so the default guidance is to use the modal's built-in props (onOk, onCancel, okText, cancelText, okButtonProps, confirmLoading) — with confirmLoading explicitly bound to the mutation's isInFlight / isPending.
  • Demoted custom footer to a last resort with three concrete justifications (third button, non-standard layout, non-button content), and kept the BAIFlex/Reset example under that bucket.
  • Added an anti-pattern block mirroring the VFolderDeployModal / ModelCardDeployModal regression spotted in PR review, where a footer was hand-rolled inside the body and confirmLoading was missed.
  • Updated the verification checklist to assert (a) OK/Cancel via props, (b) pending state via confirmLoading, (c) BAIFlex + BAIButton.action only when a custom footer is justified.

Context

PR-review thread on FR-2862 / VFolderDeployModal surfaced that the skill's prior wording (custom-footer example as the leading pattern) was nudging implementers toward reimplementing the standard footer. The Teams discussion is linked on the Jira issue.

Test plan

  • Read the updated react-modal-drawer/SKILL.md end-to-end — section 6 leads with props, custom footer is clearly framed as a last resort.
  • Verification checklist captures the new prop-first expectations.

@github-actions github-actions Bot added the size:M 30~100 LoC label May 14, 2026
Copy link
Copy Markdown
Member Author


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • flow:merge-queue - adds this PR to the back of the merge queue
  • flow:hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has required the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@yomybaby yomybaby marked this pull request as ready for review May 14, 2026 09:25
Copilot AI review requested due to automatic review settings May 14, 2026 09:25
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the react-modal-drawer skill documentation to steer implementers toward using BAIModal/antd’s built-in footer props (onOk, onCancel, okText, cancelText, okButtonProps, confirmLoading) as the default, and frames custom footer overrides as a last resort.

Changes:

  • Rewrites Section 6 to be prop-first for standard OK/Cancel behavior, including guidance to bind mutation pending state via confirmLoading.
  • Adds explicit “custom footer only when necessary” criteria and keeps the BAIFlex + BAIButton.action example under that bucket.
  • Introduces an anti-pattern example and updates the verification checklist to reflect the new guidance.

Comment thread .claude/skills/react-modal-drawer/SKILL.md Outdated
Comment thread .claude/skills/react-modal-drawer/SKILL.md
@yomybaby yomybaby force-pushed the docs/FR-2897-modal-footer-props-default branch from 1be5602 to 961abf6 Compare May 14, 2026 09:39
@yomybaby yomybaby merged commit 32dbd20 into main May 14, 2026
8 checks passed
@yomybaby yomybaby deleted the docs/FR-2897-modal-footer-props-default branch May 14, 2026 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30~100 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make modal footer props the default in react-modal-drawer skill

2 participants