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.

Sign up for GitHub

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

Jump to bottom

Support selecting multiple fragments #670

Merged
merged 3 commits into from
Mar 12, 2024
Merged

Support selecting multiple fragments #670

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a240366
Support selecting multiple fragments
joeldrapper Mar 6, 2024
575ce85
LOL
joeldrapper Mar 6, 2024
15be0d9
Don't wrap selectively rendered elements in templates
joeldrapper Mar 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 19 additions & 4 deletions lib/phlex/context.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,34 @@ def initialize(user_context = {})
@buffer = +""
@capturing = false
@user_context = user_context
@fragment = nil
@fragments = nil
@in_target_fragment = false
@found_target_fragment = false
end

attr_accessor :buffer, :capturing, :user_context, :fragment,
:in_target_fragment, :found_target_fragment
attr_accessor :buffer, :capturing, :user_context, :in_target_fragment

attr_reader :fragments

# Added for backwards compatibility with phlex-rails. We can remove this with 2.0
def target
@buffer
end

def target_fragments(fragments)
@fragments = fragments.to_h { |it| [it, true] }
end

def begin_target(id)
@in_target_fragment = id
@buffer << %(<template data-id="#{id}">)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@midnightmonster I realised we don’t return empty <template>s anyway so this data-id isn't particularly helpful.

Copy link
Contributor

Choose a reason for hiding this comment

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

We could, though. If we get all the way to the end and there's still something left in @fragments, just spit out the empty template tag.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I’m debating if this is worth it, since you can always find the missing IDs by looking at the child nodes on the client side.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It’s one of those things where if we put data-id in now, we can’t remove it later. But if we don’t, we can always add it in. So I’m inclined to keep it out for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Remembering that there's a need for both "remove this node which is no longer present" and "silly developer, why did you ask me for a node that's inside another node you also asked for?"—I think the simplest thing is not to use the template tags at all. Client side makes the request and remembers which IDs it asked for. It instantiates the response text as a DocumentFragment (or however you're doing this) and (assuming it was requesting specific IDs at all) just goes down the list of IDs it knows it requested, querying the response document fragment by ID.

  1. If it finds the ID in the response and the page, morph the page version with the new version.
  2. If it finds the ID in the response but not the page, discard with perhaps a warning (presumably it does this already?).
  3. If it finds the ID in the page but not the response, remove the node from the page.

This means it morphs a second time for a nested id, but those should be rare-to-never and it shouldn't result in any changes, so it probably doesn't matter.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This makes sense. It’s actually a not ideal to have to manage multiple <template>s in the client-side code. It would be easier to make a new <template> and set its inner HTML to whatever came back from the server, and then iterate over the template’s child elements.

end

def end_target
@fragments.delete(@in_target_fragment)
@buffer << "</template>"
@in_target_fragment = false
end

def capturing_into(new_buffer)
original_buffer = @buffer
original_capturing = @capturing
Expand Down
35 changes: 20 additions & 15 deletions lib/phlex/elements.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,18 @@ def #{method_name}(**attributes, &block)

context = @_context
buffer = context.buffer
fragment = context.fragment
end_find = false
fragment = context.fragments
target_found = false

if fragment
in_target_fragment = context.in_target_fragment
return if fragment.length == 0 # we found all our fragments already

if !in_target_fragment
if !context.found_target_fragment && attributes[:id] == fragment
context.in_target_fragment = true
context.found_target_fragment = true
end_find = true
id = attributes[:id]

if !context.in_target_fragment
if fragment[id]
context.begin_target(id)
target_found = true
else
yield(self) if block
return nil
Expand Down Expand Up @@ -83,8 +84,7 @@ def #{method_name}(**attributes, &block)

#{'flush' if tag == 'head'}

# I think we can actually throw from here.
context.in_target_fragment = false if end_find
context.end_target if target_found

nil
end
Expand Down Expand Up @@ -114,14 +114,17 @@ def #{method_name}(**attributes)
#{deprecation}
context = @_context
buffer = context.buffer
fragment = context.fragment
fragment = context.fragments

if fragment
in_target_fragment = context.in_target_fragment
return if fragment.length == 0 # we found all our fragments already

id = attributes[:id]

if !in_target_fragment
if !context.found_target_fragment && attributes[:id] == fragment
context.found_target_fragment = true
if !context.in_target_fragment
if fragment[id]
context.begin_target(id)
target_found = true
else
return nil
end
Expand All @@ -134,6 +137,8 @@ def #{method_name}(**attributes)
buffer << "<#{tag}>"
end

context.end_target if target_found

nil
end

Expand Down
2 changes: 1 addition & 1 deletion lib/phlex/html.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def __unbuffered_class__
# Output an HTML doctype.
def doctype
context = @_context
return if context.fragment && !context.in_target_fragment
return if context.fragments && !context.in_target_fragment

context.buffer << "<!DOCTYPE html>"
nil
Expand Down
12 changes: 6 additions & 6 deletions lib/phlex/sgml.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,12 @@ def call(...)
end

# @api private
def __final_call__(buffer = +"", context: Phlex::Context.new, view_context: nil, parent: nil, fragment: nil, &block)
def __final_call__(buffer = +"", context: Phlex::Context.new, view_context: nil, parent: nil, fragments: nil, &block)
@_buffer = buffer
@_context = context
@_view_context = view_context
@_parent = parent
@_context.fragment = fragment if fragment
@_context.target_fragments(fragments) if fragments

block ||= @_content_block

Expand Down Expand Up @@ -147,7 +147,7 @@ def context
# @see #format_object
def plain(content)
context = @_context
return if context.fragment && !context.in_target_fragment
return if context.fragments && !context.in_target_fragment

unless __text__(content)
raise ArgumentError, "You've passed an object to plain that is not handled by format_object. See https://rubydoc.info/gems/phlex/Phlex/SGML#format_object-instance_method for more information"
Expand All @@ -161,7 +161,7 @@ def plain(content)
# @yield If a block is given, it yields the block with no arguments.
def whitespace(&block)
context = @_context
return if context.fragment && !context.in_target_fragment
return if context.fragments && !context.in_target_fragment

buffer = context.buffer

Expand All @@ -179,7 +179,7 @@ def whitespace(&block)
# @return [nil]
def comment(&block)
context = @_context
return if context.fragment && !context.in_target_fragment
return if context.fragments && !context.in_target_fragment

buffer = context.buffer

Expand All @@ -197,7 +197,7 @@ def unsafe_raw(content = nil)
return nil unless content

context = @_context
return if context.fragment && !context.in_target_fragment
return if context.fragments && !context.in_target_fragment

context.buffer << content
nil
Expand Down
17 changes: 12 additions & 5 deletions test/phlex/selective_rendering.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ def view_template
strong { "World" }
img(src: "image.jpg")
}
img(src: "after.jpg")
strong { "Here" }
img(id: "image", src: "after.jpg")
h1(id: "target") { "After" }
}
end
Expand Down Expand Up @@ -43,13 +44,19 @@ def view_template
describe Phlex::HTML do
it "renders the just the target fragment" do
expect(
StandardElementExample.new.call(fragment: "target")
).to be == %(<h1 id="target">Hello<strong>World</strong><img src="image.jpg"></h1>)
StandardElementExample.new.call(fragments: ["target"])
).to be == %(<template data-id="target"><h1 id="target">Hello<strong>World</strong><img src="image.jpg"></h1></template>)
end

it "works with void elements" do
expect(
VoidElementExample.new.call(fragment: "target")
).to be == %(<img id="target" src="image.jpg">)
VoidElementExample.new.call(fragments: ["target"])
).to be == %(<template data-id="target"><img id="target" src="image.jpg"></template>)
end

it "supports multiple fragments" do
expect(
StandardElementExample.new.call(fragments: ["target", "image"])
).to be == %(<template data-id="target"><h1 id="target">Hello<strong>World</strong><img src="image.jpg"></h1></template><template data-id="image"><img id="image" src="after.jpg"></template>)
end
end