Skip to content
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

Add ParentNode and fix children implementation #46

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions opal/browser/dom.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require 'browser/dom/node_set'
require 'browser/dom/node'
require 'browser/dom/parent_node'
require 'browser/dom/attribute'
require 'browser/dom/character_data'
require 'browser/dom/text'
Expand Down
2 changes: 2 additions & 0 deletions opal/browser/dom/element.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
module Browser; module DOM

class Element < Node
include ParentNode

def self.create(*args)
$document.create_element(*args)
end
Expand Down
41 changes: 9 additions & 32 deletions opal/browser/dom/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,6 @@ def remove
parent.remove_child(self) if parent
end

# Remove all the children of the node.
def clear
children.remove
end

# @!attribute content
# @return [String] the inner text content of the node
Copy link
Member

Choose a reason for hiding this comment

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

Was this removed on purpose? I still want that available.

Copy link
Author

Choose a reason for hiding this comment

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

I removed it because I wasn't sure if it will work as children is read-only, but seems individual elements can still be removed. Also previously it would remove all child nodes, but now this would remove only element nodes. What is expected behavior?

if Browser.supports? 'Element.textContent'
Expand Down Expand Up @@ -235,17 +230,19 @@ def cdata?
# @!attribute [r] child
# @return [Node?] the first child of the node
def child
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this and all the children related methods in ParentNode now?

Also I'm not entirely sure I like the separation of that into a module, what's the rationale behind it?

Copy link
Author

Choose a reason for hiding this comment

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

Currently in DOM spec for ParentNode there are only 4 attributes defined and other attributes like firstChild, lastChild are defined for Node and not for ParentNode so I'm not really sure, but it can be moved.

I think implementation should follow as close to DOM spec as possible (while also giving convenience methods) and there's ParentNode Interface so this follows it.

EDIT: Also I think it makes sense why they created such interface because there can be Nodes which can't have any element children.

children.first
first_child
end

# @!attribute children
# @return [NodeSet] the children of the node
def children
NodeSet[Native::Array.new(`#@native.childNodes`)]
# @!attribute [r] first_child
# @return [Node?] the first child of the node
def first_child
DOM(`#@native.firstChild`)
end

def children=(node)
raise NotImplementedError
# @!attribute [r] last_child
# @return [Node?] the last child of the node
def last_child
DOM(`#@native.lastChild`)
end

# Return true if the node is a comment.
Expand All @@ -271,20 +268,6 @@ def elem?

alias element? elem?

# @!attribute [r] element_children
# @return [NodeSet] all the children which are elements
def element_children
children.select(&:element?)
end

alias elements element_children

# @!attribute [r] first_element_child
# @return [Element?] the first element child
def first_element_child
element_children.first
end

# Return true if the node is a document fragment.
def fragment?
node_type == DOCUMENT_FRAGMENT_NODE
Expand All @@ -303,12 +286,6 @@ def inner_html=(value)
alias inner_text content
alias inner_text= content=

# @!attribute [r] last_element_child
# @return [Element?] the last element child
def last_element_child
element_children.last
end

# @!attribute name
# @return [String] the name of the node
def name
Expand Down
36 changes: 36 additions & 0 deletions opal/browser/dom/parent_node.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
module Browser; module DOM

# Abstract class for all Nodes that can have children.
#
# @see https://developer.mozilla.org/en-US/docs/Web/API/ParentNode
class ParentNode

# @!attribute [r] children
# @return [NodeSet] the children of the node
def children
NodeSet[Native::Array.new(`#@native.children`)]
end

alias elements children

# @!attribute [r] first_element_child
# @return [Element?] the first element child
def first_element_child
DOM(`#@native.firstElementChild`)
end

# @!attribute [r] last_element_child
# @return [Element?] the last element child
def last_element_child
DOM(`#@native.lastElementChild`)
end

# @!attribute [r] child_element_count
# @return [Element?] the last element child
def child_element_count
`#@native.childElementCount`
end

end

end; end