Skip to content

Address follow-up review issues in pagination, overlay teardown, and tooltip/select/textarea behavior#13

Merged
JosunLP merged 8 commits intodevfrom
copilot/sub-pr-11
Mar 25, 2026
Merged

Address follow-up review issues in pagination, overlay teardown, and tooltip/select/textarea behavior#13
JosunLP merged 8 commits intodevfrom
copilot/sub-pr-11

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 24, 2026

  • Inspect the new review comment about getAnimationTimeoutMs() returning non-fallback timeouts when animation-name is none
  • Check recent GitHub Actions workflow runs for dev, then copilot/sub-pr-11 if needed, to confirm whether there are relevant CI failures
  • Run targeted validation for the animation-timeout helper and affected overlay/accordion paths
  • Apply the smallest safe fix so animation-name: none always uses the fallback timeout while named 0ms animations still return 0
  • Re-run targeted tests and any necessary type/build checks
  • Run final review and security checks, then reply on the new PR comment with the addressing commit

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: JosunLP <20913954+JosunLP@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bQuery/ui/sessions/c5a423e6-fdec-4d0e-8f0b-7330cee6f36f
Copilot AI changed the title [WIP] Update documentation for UI components accessibility Address follow-up review issues in pagination, overlay teardown, and tooltip/select/textarea behavior Mar 24, 2026
Copilot AI requested a review from JosunLP March 24, 2026 22:22
@JosunLP JosunLP marked this pull request as ready for review March 24, 2026 22:22
Copilot AI review requested due to automatic review settings March 24, 2026 22:22
Copy link
Copy Markdown

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

This PR tightens a set of edge cases across UI components on the 1.0.0 branch, focusing on pagination validity, animation-aligned overlay teardown, disconnect safety, and a small markup fix.

Changes:

  • Clamp rendered pagination page to a valid range to keep current-page rendering stable when page is out of bounds.
  • Introduce/reuse a shared getAnimationTimeoutMs() utility and adopt it for close/teardown timing in overlay-like components; add timer cleanup on disconnect and tests for these cases.
  • Improve disconnect safety for tooltip deferred listener setup; remove redundant select sync; fix malformed textarea closing tag; refactor accordion to use the shared animation-timeout helper.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/tabs-tooltip.test.ts Adds regression test for tooltip disconnect before queued microtask runs.
tests/slider-pagination.test.ts Adds regression test for pagination rendering when page is out of range.
tests/overlay-components.test.ts Adds tests ensuring dialog/drawer close timers don’t fire after disconnect.
tests/dropdown-menu.test.ts Adds test ensuring dropdown-menu close timer doesn’t fire after disconnect.
src/utils/dom.ts Adds shared getAnimationTimeoutMs() helper for computed animation duration/delay.
src/components/tooltip/BqTooltip.ts Guards microtask work with isConnected to avoid stale listener attachment.
src/components/textarea/BqTextarea.ts Fixes malformed <textarea> closing tag in rendered markup.
src/components/select/BqSelect.ts Removes redundant _syncValue call now that option sync schedules reconciliation.
src/components/pagination/BqPagination.ts Clamps rendered page value to [1, total] range.
src/components/dropdown-menu/BqDropdownMenu.ts Uses computed animation timeout for close and clears pending close timer on disconnect.
src/components/drawer/BqDrawer.ts Uses computed animation timeout for close and clears pending close timer on disconnect.
src/components/dialog/BqDialog.ts Uses computed animation timeout for close and clears pending close timer on disconnect.
src/components/accordion/BqAccordion.ts Switches accordion close fallback timing to shared getAnimationTimeoutMs().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@JosunLP
Copy link
Copy Markdown
Contributor

JosunLP commented Mar 25, 2026

@copilot apply changes based on the comments in this thread

Copilot AI and others added 3 commits March 25, 2026 07:16
Co-authored-by: JosunLP <20913954+JosunLP@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bQuery/ui/sessions/4fc6490e-33d3-41ae-b7c5-f770d1b456ae
Co-authored-by: JosunLP <20913954+JosunLP@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bQuery/ui/sessions/4fc6490e-33d3-41ae-b7c5-f770d1b456ae
Co-authored-by: JosunLP <20913954+JosunLP@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bQuery/ui/sessions/4fc6490e-33d3-41ae-b7c5-f770d1b456ae
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 25, 2026

@copilot apply changes based on the comments in this thread

Applied the disconnect cleanup follow-up in 6e58bb4. Pending close teardown now clears transient open/data-closing state for dialog, drawer, and dropdown-menu, with regression coverage for the disconnect path. Screenshot: N/A — this is a non-visual runtime/state fix.

Copy link
Copy Markdown

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

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@JosunLP
Copy link
Copy Markdown
Contributor

JosunLP commented Mar 25, 2026

@copilot apply changes based on the comments in this thread

Co-authored-by: JosunLP <20913954+JosunLP@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bQuery/ui/sessions/c5806ac5-914b-4ba5-af38-b97bca44f7df
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 25, 2026

@copilot apply changes based on the comments in this thread

Applied in 90967bb. Removed the unused syncValue wrapper and _syncValue stored field from bq-select, then re-ran the targeted select test plus type/build checks. Screenshot: N/A — this is a non-visual cleanup.

Copy link
Copy Markdown

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

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@JosunLP
Copy link
Copy Markdown
Contributor

JosunLP commented Mar 25, 2026

@copilot apply changes based on the comments in this thread

Co-authored-by: JosunLP <20913954+JosunLP@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bQuery/ui/sessions/632cae1e-162b-4168-ae82-7c2a4e7e2276
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 25, 2026

@copilot apply changes based on the comments in this thread

Applied in c221caa. getAnimationTimeoutMs() now honors named 0ms animations while still falling back when computed animation timing/name data is missing or empty, and I added focused regression coverage plus revalidated the accordion timeout path. Screenshot: N/A — this is a non-visual timing fix.

Copy link
Copy Markdown

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

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@JosunLP
Copy link
Copy Markdown
Contributor

JosunLP commented Mar 25, 2026

@copilot apply changes based on the comments in this thread

Co-authored-by: JosunLP <20913954+JosunLP@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bQuery/ui/sessions/50f1c1d0-7fe3-49f3-9dd3-cb1614d65271
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 25, 2026

@copilot apply changes based on the comments in this thread

Applied in 3870698. getAnimationTimeoutMs() now falls back whenever animation-name is not a real animation, and the focused utility plus accordion timing tests were updated to cover that contract. Screenshot: N/A — this is a non-visual timing fix.

Copy link
Copy Markdown

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

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@JosunLP JosunLP merged commit f7a5ea7 into dev Mar 25, 2026
5 checks passed
@JosunLP JosunLP deleted the copilot/sub-pr-11 branch March 25, 2026 09:45
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