-
Notifications
You must be signed in to change notification settings - Fork 0
Enhance DOM utilities: support HTMLElement inputs, detailed feedback, and safer handling #19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ors correctly, and support HTMLElement input
…mplete callback, and handle errors
There was a problem hiding this 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 enhances several DOM utility functions to accept both CSS selectors and direct HTMLElements, improves safety by returning null or empty arrays when targets aren’t found, and provides detailed feedback and error handling in focus and scroll helpers.
- Support for passing HTMLElements alongside selectors in all utilities
- Safe null handling: return
nullor[]instead of throwing - Detailed result objects (
focusElement) and callbacks (scrollToElementAfterRender) with error info
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/dom/toggleInertAround.ts | Accepts HTMLElement inputs and safely handles null parent elements |
| src/dom/scrollToElementAfterRender.ts | Accepts selectors/HTMLElements, adds onScrollComplete callback, error handling |
| src/dom/isElementInViewport.ts | Accepts selectors or elements, returns false if not found |
| src/dom/getFocusableElements.ts | Supports selector or element containers, returns empty array if none |
| src/dom/getElementDimensions.ts | Accepts selector or element, returns null if not found |
| src/dom/focusElement.ts | Accepts selectors/HTMLElements, returns detailed focus result objects |
Comments suppressed due to low confidence (3)
src/dom/scrollToElementAfterRender.ts:2
- Update the description to reference CSS selectors instead of 'ID', since the function now accepts selectors or HTMLElements (e.g., 'specified CSS selector or HTMLElement').
* Scrolls the element with the specified ID or element reference into view after the next render frame.
src/dom/tests/scrollToElementAfterRender.test.ts:59
- Consider adding a test to verify that
onScrollCompleteis invoked withelement: nulland no error when the selector matches no element.
it('does nothing if element is not found via selector', () => {
src/dom/getFocusableElements.ts:41
- [nitpick] Consider extracting the repeated "string vs HTMLElement" resolution logic into a shared helper to reduce duplication across DOM utilities.
if (typeof containerOrSelector === 'string') {
…ment resolution Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… CSS selectors instead of 'ID'
Changes
nullor empty arrays when target elements are not found, avoiding errorsfocusElementto return a detailed result object with focus attempt status and error infoscrollToElementAfterRenderaccept HTMLElements and handle errors with callback feedbacktoggleInertAroundto prevent runtime errors