feat: implement removal methods for HTMLElement and add tests - #20
Merged
Conversation
ziflex
added
enhancement
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds child-removal support to HTMLElement implementations across the memory and CDP backends, exposing removal by index (RemoveAt) and by key (RemoveKey) and updating capability/contract tests accordingly.
Changes:
- Implement
RemoveAt/RemoveKeyonmemory.HTMLElementandcdpdom.HTMLElementto remove child elements by index. - Add a CDP JS evaluation template (
removeChildByIndex) used by CDP-backed element removal. - Extend capability/contract tests and add new unit tests (notably for memory-backed removal and key-type validation).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/web/html/drivers/read_only_contract_test.go | Extends contract assertions to include IndexRemovable/KeyRemovable support expectations. |
| modules/web/html/drivers/memory/element.go | Adds child removal implementations for memory-backed elements. |
| modules/web/html/drivers/memory/element_removal_test.go | Adds unit tests validating memory-backed removal behavior and edge cases. |
| modules/web/html/drivers/cdp/templates/children.go | Introduces JS template used to remove a child by index in the CDP backend. |
| modules/web/html/drivers/cdp/dom/element_value_test.go | Adds a unit test to ensure RemoveKey rejects non-integer keys. |
| modules/web/html/drivers/cdp/dom/element_query.go | Adds CDP-backed RemoveAt/RemoveKey methods using the new template. |
| modules/web/html/drivers/capabilities_test.go | Registers and validates IndexRemovable/KeyRemovable capabilities for both backends. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
18
to
+20
| keyWritable := reflect.TypeOf((*runtime.KeyWritable)(nil)).Elem() | ||
| indexRemovable := reflect.TypeOf((*runtime.IndexRemovable)(nil)).Elem() | ||
| keyRemovable := reflect.TypeOf((*runtime.KeyRemovable)(nil)).Elem() |
Comment on lines
+20
to
+36
| func (el *HTMLElement) RemoveAt(ctx context.Context, idx runtime.Int) (runtime.Value, error) { | ||
| if idx < 0 { | ||
| return runtime.None, nil | ||
| } | ||
|
|
||
| return el.eval.EvalElement(ctx, templates.RemoveChildByIndex(el.id, idx)) | ||
| } | ||
|
|
||
| func (el *HTMLElement) RemoveKey(ctx context.Context, key runtime.Value) error { | ||
| idx, ok := key.(runtime.Int) | ||
| if !ok { | ||
| return runtime.Error(runtime.ErrInvalidArgument, "element child index must be an integer") | ||
| } | ||
|
|
||
| _, err := el.RemoveAt(ctx, idx) | ||
|
|
||
| return err |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces support for child element removal by index and key to both memory and CDP-backed HTML elements, along with comprehensive tests to verify this new functionality. The changes ensure that
HTMLElementtypes in both backends implement the newIndexRemovableandKeyRemovableinterfaces, and that removal operations behave consistently and correctly, including type safety for keys.New removal functionality for HTML elements:
RemoveAtandRemoveKeymethods tomemory.HTMLElementandcdpdom.HTMLElement, allowing removal of child elements by index and key. These methods return the removed element or an error if the operation is invalid (e.g., non-integer key). [1] [2]removeChildByIndex) and integrated it into the element evaluation logic.Interface and capability updates:
IndexRemovableandKeyRemovableinterfaces for both backends and updated capability tests to verify their implementation. [1] [2] [3] [4] [5]Testing improvements:
RemoveKeyrejects non-integer keys. [1] [2]These changes collectively enhance the flexibility and robustness of HTML element manipulation in both memory and CDP drivers.