From fe6193a1c4f2499d4873679ba9dd4fa9f9f95580 Mon Sep 17 00:00:00 2001 From: Noah Gibbs Date: Thu, 26 Oct 2023 10:34:47 +0100 Subject: [PATCH] Wrap rendered drawables in an outer div so it can be replaced or removed fully. Don't do this for TextDrawables - they're tiny. Make Link and Span be TextDrawables, which they should have been anyway. Default needs_update to just updating the single drawable, not the whole tree. This is related to #419, but doesn't fully fix it. --- lib/scarpe/evented_assertions.rb | 19 +++++++ lib/scarpe/wv.rb | 2 +- lib/scarpe/wv/drawable.rb | 50 +++++++++++++------ lib/scarpe/wv/link.rb | 2 +- lib/scarpe/wv/span.rb | 2 +- lib/scarpe/wv/text_drawable.rb | 4 ++ lib/scarpe/wv/web_wrangler.rb | 10 +++- .../lib/scarpe/components/calzini.rb | 20 +++++++- test/test_edit_box.rb | 6 +-- test/test_helper.rb | 19 +++++++ test/test_image.rb | 13 +++-- 11 files changed, 116 insertions(+), 31 deletions(-) diff --git a/lib/scarpe/evented_assertions.rb b/lib/scarpe/evented_assertions.rb index a1f2e6c1a..80eebae38 100644 --- a/lib/scarpe/evented_assertions.rb +++ b/lib/scarpe/evented_assertions.rb @@ -90,6 +90,25 @@ def assert_html(actual_html, expected_tag, **opts, &block) assert_equal expected_html, actual_html end + # Assert that `actual_html` includes `expected_tag` with `opts`. + # This uses Scarpe's HTML tag-based renderer to render the tag and options + # into text, and valides that the full HTML contains that tag. + # + # @see Scarpe::Components::HTML.render + # + # @param actual_html [String] the html to compare to + # @param expected_tag [String,Symbol] the HTML tag, used to send a method call + # @param opts keyword options passed to the tag method call + # @yield block passed to the tag method call. + # @return [void] + def assert_contains_html(actual_html, expected_tag, **opts, &block) + expected_html = Scarpe::Components::HTML.render do |h| + h.public_send(expected_tag, opts, &block) + end + + assert_include actual_html, expected_html + end + def return_assertion_data if !@assertions_failed.empty? return_results(false, "Assertions failed", assertion_data_as_a_struct) diff --git a/lib/scarpe/wv.rb b/lib/scarpe/wv.rb index 601a8367b..02c9c7d89 100644 --- a/lib/scarpe/wv.rb +++ b/lib/scarpe/wv.rb @@ -70,10 +70,10 @@ module Scarpe::Webview require_relative "wv/edit_line" require_relative "wv/list_box" require_relative "wv/alert" -require_relative "wv/span" require_relative "wv/shape" require_relative "wv/text_drawable" +require_relative "wv/span" require_relative "wv/link" require_relative "wv/line" require_relative "wv/rect" diff --git a/lib/scarpe/wv/drawable.rb b/lib/scarpe/wv/drawable.rb index 531d5d0d1..565f99377 100644 --- a/lib/scarpe/wv/drawable.rb +++ b/lib/scarpe/wv/drawable.rb @@ -99,11 +99,11 @@ def properties_changed(changes) if changes.key?("hidden") hidden = changes.delete("hidden") if hidden - html_element.set_style("display", "none") + html_wrapper_element.set_style("display", "none") else new_style = style # Get current display CSS property, which may vary by subclass disp = new_style[:display] - html_element.set_style("display", disp || "block") + html_wrapper_element.set_style("display", disp || "block") end end @@ -180,18 +180,32 @@ def style public - # This gets a mini-webview for just this element and its children, if any. + # This gets an accessor for just this element's HTML ID. # It is normally called by the drawable itself to do its DOM management. + # Some drawables don't use their html_id for their outermost element, + # and so we add an outer wrapping div to make sure that remove(), + # hidden() etc. affect every part of the drawable. This accessor + # does *not* necessarily affect the outermost elements. # # @return [Scarpe::WebWrangler::ElementWrangler] a DOM object manager def html_element @elt_wrangler ||= Scarpe::Webview::WebWrangler::ElementWrangler.new(html_id) end + # This gets an accessor for just this element's outer wrapping div. + # This allows replacing the entire drawable, not just its "main" element. + # + # @return [Scarpe::WebWrangler::ElementWrangler] a DOM object manager + def html_wrapper_element + @elt_wrangler ||= Scarpe::Webview::WebWrangler::ElementWrangler.new(html_id) + end + # Return a promise that guarantees all currently-requested changes have completed # # @return [Scarpe::Promise] a promise that will be fulfilled when all pending changes have finished def promise_update + # Doesn't matter what ElementWrangler we use -- they all return an update promise + # that includes all pending updates, no matter who they're for. html_element.promise_update end @@ -199,7 +213,7 @@ def promise_update # # @return [String] the HTML ID def html_id - object_id.to_s + @linkable_id.to_s end # to_html is intended to get the HTML DOM rendering of this object and its children. @@ -209,11 +223,7 @@ def html_id def to_html @children ||= [] child_markup = @children.map(&:to_html).join - if respond_to?(:element) - element { child_markup } - else - child_markup - end + "
" + element { child_markup } + "
" end # This binds a Scarpe JS callback, handled via a single dispatch point in the app @@ -234,18 +244,28 @@ def bind(event, &block) def destroy_self @parent&.remove_child(self) unsub_all_shoes_events - html_element.remove + html_wrapper_element.remove end - # Request a full redraw of all drawables. + # Request a full redraw of the entire window, including the entire tree of + # drawables and the outer "empty page" frame. # - # It's really hard to do dirty-tracking here because the redraws are fully asynchronous. - # And so we can't easily cancel one "in flight," and we can't easily pick up the latest - # changes... And we probably don't want to, because we may be halfway through a batch. + # @return [void] + def full_window_redraw! + DisplayService.instance.app.request_redraw! + end + + # Request a full redraw of this drawable, including all its children. + # Can be overridden in drawable subclasses if needed. An override would normally + # only be needed if re-rendering the element with the given html_id + # wasn't enough (and then remove would also need to be overridden.) + # + # This occurs by default if a property is changed and the drawable + # doesn't remove its change in property_changed. # # @return [void] def needs_update! - DisplayService.instance.app.request_redraw! + html_wrapper_element.outer_html = to_html end # Generate JS code to trigger a specific event name on this drawable with the supplies arguments. diff --git a/lib/scarpe/wv/link.rb b/lib/scarpe/wv/link.rb index 06a0f8214..4fa41c5fa 100644 --- a/lib/scarpe/wv/link.rb +++ b/lib/scarpe/wv/link.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Scarpe::Webview - class Link < Drawable + class Link < TextDrawable def initialize(properties) super diff --git a/lib/scarpe/wv/span.rb b/lib/scarpe/wv/span.rb index 222bb55a8..0a8011672 100644 --- a/lib/scarpe/wv/span.rb +++ b/lib/scarpe/wv/span.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Scarpe::Webview - class Span < Drawable + class Span < TextDrawable SIZES = { inscription: 10, ins: 10, diff --git a/lib/scarpe/wv/text_drawable.rb b/lib/scarpe/wv/text_drawable.rb index 67ead3e7f..946a57f48 100644 --- a/lib/scarpe/wv/text_drawable.rb +++ b/lib/scarpe/wv/text_drawable.rb @@ -2,6 +2,10 @@ module Scarpe::Webview class TextDrawable < Drawable + def to_html + # Do not render TextDrawables with individual wrapper divs. + element + end end class << self diff --git a/lib/scarpe/wv/web_wrangler.rb b/lib/scarpe/wv/web_wrangler.rb index e1abb60ff..c657348ca 100644 --- a/lib/scarpe/wv/web_wrangler.rb +++ b/lib/scarpe/wv/web_wrangler.rb @@ -785,7 +785,15 @@ def inner_html=(new_html) @webwrangler.dom_change("document.getElementById(\"" + html_id + "\").innerHTML = `" + new_html + "`; true") end - # Update the JS DOM element's inner_html. The given Ruby value will be inspected and assigned. + # Update the JS DOM element's outer_html. The given Ruby value will be converted to string and assigned in backquotes. + # + # @param new_html [String] the new outer_html + # @return [Scarpe::Promise] a promise that will be fulfilled when the change is complete + def outer_html=(new_html) + @webwrangler.dom_change("document.getElementById(\"" + html_id + "\").outerHTML = `" + new_html + "`; true") + end + + # Update the JS DOM element's attribute. The given Ruby value will be inspected and assigned. # # @param attribute [String] the attribute name # @param value [String] the new attribute value diff --git a/scarpe-components/lib/scarpe/components/calzini.rb b/scarpe-components/lib/scarpe/components/calzini.rb index 4b4d41302..bb1f68347 100644 --- a/scarpe-components/lib/scarpe/components/calzini.rb +++ b/scarpe-components/lib/scarpe/components/calzini.rb @@ -33,10 +33,26 @@ module Scarpe::Components::Calzini }.freeze private_constant :SIZES - def render(drawable, properties = shoes_styles, &block) - send("#{drawable}_element", properties, &block) + # Render the Shoes drawable of type `drawable_name` with + # the given properties to HTML and return it. If the + # drawable type takes a block (e.g. Stack or Flow) then + # the block will be properly rendered. + # + # @param drawable_name [String] the drawable name like "alert", "button" or "rect" + # @param properties [Hash] a drawable-specific hash of property names to values + # @block the block which, when called, will return the contents for drawable types with contents + # @return [String] the rendered HTML + def render(drawable_name, properties = shoes_styles, &block) + send("#{drawable_name}_element", properties, &block) end + # Return HTML for an empty page element, to be filled with HTML + # renderings of the DOM tree. + # + # The wrapper-wvroot element is where Scarpe will fill in the + # DOM element. + # + # @return [String] the rendered HTML for the empty page object. def empty_page_element <<~HTML diff --git a/test/test_edit_box.rb b/test/test_edit_box.rb index cfe917ce5..14d5833c0 100644 --- a/test/test_edit_box.rb +++ b/test/test_edit_box.rb @@ -14,7 +14,7 @@ def test_renders_textarea on_heartbeat do box = edit_box html_id = box.display.html_id - assert_html edit_box.display.to_html, :textarea, id: html_id, oninput: "scarpeHandler('#{box.display.shoes_linkable_id}-change', this.value)" do + assert_contains_html edit_box.display.to_html, :textarea, id: html_id, oninput: "scarpeHandler('#{box.display.shoes_linkable_id}-change', this.value)" do "Hello, World!" end @@ -35,7 +35,7 @@ def test_renders_textarea_no_change_cb_on_manual_replace box.text = "Awwww yeah" wait fully_updated html_id = box.display.html_id - assert_html edit_box.display.to_html, :textarea, id: html_id, oninput: "scarpeHandler('#{box.display.shoes_linkable_id}-change', this.value)" do + assert_contains_html edit_box.display.to_html, :textarea, id: html_id, oninput: "scarpeHandler('#{box.display.shoes_linkable_id}-change', this.value)" do "Awwww yeah" end # Shoes3 does *not* fire a change event when manually replacing text @@ -55,7 +55,7 @@ def test_textarea_width on_heartbeat do box = edit_box html_id = box.display.html_id - assert_html edit_box.display.to_html, + assert_contains_html edit_box.display.to_html, :textarea, id: html_id, oninput: "scarpeHandler('#{box.display.shoes_linkable_id}-change', this.value)", diff --git a/test/test_helper.rb b/test/test_helper.rb index 14d0d0aba..b91ce4e1c 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -176,6 +176,25 @@ def assert_html(actual_html, expected_tag, **opts, &block) assert_equal expected_html, actual_html end + + # Assert that `actual_html` includes `expected_tag` with `opts`. + # This uses Scarpe's HTML tag-based renderer to render the tag and options + # into text, and valides that the full HTML contains that tag. + # + # @see Scarpe::Components::HTML.render + # + # @param actual_html [String] the html to compare to + # @param expected_tag [String,Symbol] the HTML tag, used to send a method call + # @param opts keyword options passed to the tag method call + # @yield block passed to the tag method call. + # @return [void] + def assert_contains_html(actual_html, expected_tag, **opts, &block) + expected_html = Scarpe::Components::HTML.render do |h| + h.public_send(expected_tag, opts, &block) + end + + assert_includes actual_html, expected_html + end end class LoggedScarpeTest < ScarpeWebviewTest diff --git a/test/test_image.rb b/test/test_image.rb index d053a97af..90a38e716 100644 --- a/test/test_image.rb +++ b/test/test_image.rb @@ -19,7 +19,7 @@ def teardown def test_renders_image img = Scarpe::Webview::Image.new(@default_properties) - assert_html img.to_html, :img, id: img.html_id, src: @url + assert_contains_html img.to_html, :img, id: img.html_id, src: @url end def test_renders_image_with_specified_size @@ -27,7 +27,7 @@ def test_renders_image_with_specified_size height = 50 img = Scarpe::Webview::Image.new(@default_properties.merge(width:, height:)) - assert_html img.to_html, :img, id: img.html_id, src: @url, style: "width:#{width}px;height:#{height}px" + assert_contains_html img.to_html, :img, id: img.html_id, src: @url, style: "width:#{width}px;height:#{height}px" end def test_renders_image_with_specified_position @@ -35,7 +35,7 @@ def test_renders_image_with_specified_position left = 5 img = Scarpe::Webview::Image.new(@default_properties.merge(top:, left:)) - assert_html img.to_html, :img, id: img.html_id, src: @url, style: "top:#{top}px;left:#{left}px;position:absolute" + assert_contains_html img.to_html, :img, id: img.html_id, src: @url, style: "top:#{top}px;left:#{left}px;position:absolute" end def test_renders_image_with_specified_size_and_position @@ -45,7 +45,7 @@ def test_renders_image_with_specified_size_and_position left = 5 img = Scarpe::Webview::Image.new(@default_properties.merge(width:, height:, top:, left:)) - assert_html img.to_html, + assert_contains_html img.to_html, :img, id: img.html_id, src: @url, @@ -56,10 +56,9 @@ def test_renders_clickable_image target_url = "http://github.com/schwad/scarpe" img = Scarpe::Webview::Image.new(@default_properties.merge("click" => target_url)) - assert_equal ""\ + assert_includes img.to_html, ""\ ""\ - "", - img.to_html + "" end def test_image_size