Stop scrolling to top when opening dialog (#215)

* Stop scrolling to top when opening dialog

Also increase top offset to keep page context visible

* Fix scrolling to top when dialog opens on mobile

* Use device classes to set position

* Use platform_agent to set platform classes

* Set platform classes on fieldset

Gemfile.lock

@@ -2,6 +2,7 @@ PATH
   remote: .
   specs:
     hotwire_combobox (0.3.2)
+      platform_agent (>= 1.0.1)
       rails (>= 7.0.7.2)
       stimulus-rails (>= 1.2)
       turbo-rails (>= 1.2)
@@ -157,6 +158,9 @@ GEM
     parser (3.3.5.0)
       ast (~> 2.4.1)
       racc
+    platform_agent (1.0.1)
+      activesupport (>= 5.2.0)
+      useragent (~> 0.16.3)
     propshaft (1.1.0)
       actionpack (>= 7.0.0)
       activesupport (>= 7.0.0)

app/assets/javascripts/hotwire_combobox.esm.js

@@ -1618,6 +1618,7 @@ Combobox.Toggle = Base => class extends Base {
     this._preventFocusingComboboxAfterClosingDialog();
     this._preventBodyScroll();
     this.dialogTarget.showModal();
+    this._resizeDialog();
   }
 
   _openInline() {

app/assets/javascripts/hw_combobox/models/combobox/toggle.js

@@ -121,6 +121,7 @@ Combobox.Toggle = Base => class extends Base {
     this._preventFocusingComboboxAfterClosingDialog()
     this._preventBodyScroll()
     this.dialogTarget.showModal()
+    this._resizeDialog()
   }
 
   _openInline() {

app/assets/stylesheets/hotwire_combobox.css

@@ -23,7 +23,7 @@
   --hw-dialog-label-size: 1.05rem;
   --hw-dialog-listbox-margin: 1.25rem 0 0;
   --hw-dialog-padding: 1rem 1rem 0;
-  --hw-dialog-top-offset: 4rem;
+  --hw-dialog-top-offset: 18vh;
 
   --hw-font-size: 1rem;
 
@@ -203,7 +203,7 @@
   overflow: hidden;
   padding: var(--hw-dialog-padding);
   pointer-events: auto;
-  position: absolute;
+  position: fixed;
   top: var(--hw-dialog-top-offset);
   width: auto;
 
@@ -217,6 +217,10 @@
   &::backdrop {
     background-image: linear-gradient(to bottom, rgba(0, 0, 0, 0.6) 50%, white 50%);
   }
+
+  .hw-combobox--ios & {
+    position: absolute;
+  }
 }
 
 .hw-combobox__dialog__label {

app/presenters/hotwire_combobox/component.rb

@@ -26,10 +26,11 @@ def initialize(
     name_when_new: nil,
     open: false,
     options: [],
+    request: nil,
     value: nil, **rest)
-    @view, @autocomplete, @id, @name, @value, @form, @async_src, @label, @free_text,
+    @view, @autocomplete, @id, @name, @value, @form, @async_src, @label, @free_text, @request,
     @name_when_new, @open, @data, @mobile_at, @multiselect_chip_src, @options, @dialog_label =
-      view, autocomplete, id, name.to_s, value, form, async_src, label, free_text,
+      view, autocomplete, id, name.to_s, value, form, async_src, label, free_text, request,
       name_when_new, open, data, mobile_at, multiselect_chip_src, options, dialog_label
 
     @combobox_attrs = input.reverse_merge(rest).deep_symbolize_keys
@@ -44,7 +45,7 @@ def render_in(view_context, &block)
   end
 
   private
-    attr_reader :view, :autocomplete, :id, :name, :value, :form, :free_text, :open,
+    attr_reader :view, :autocomplete, :id, :name, :value, :form, :free_text, :open, :request,
       :data, :combobox_attrs, :mobile_at, :association_name, :multiselect_chip_src
 
     def canonical_id

app/presenters/hotwire_combobox/component/markup/fieldset.rb

@@ -1,10 +1,19 @@
 module HotwireCombobox::Component::Markup::Fieldset
   def fieldset_attrs
     customize :fieldset, base: {
-      class: [ "hw-combobox", { "hw-combobox--multiple": multiselect? } ], data: fieldset_data }
+      class: [ "hw-combobox", platform_classes, { "hw-combobox--multiple": multiselect? } ], data: fieldset_data }
   end
 
   private
+    def platform_classes
+      platform = HotwireCombobox::Platform.new request&.user_agent
+
+      view.token_list \
+        "hw-combobox--ios": platform.ios?,
+        "hw-combobox--android": platform.android?,
+        "hw-combobox--mobile-webkit": platform.mobile_webkit?
+    end
+
     def fieldset_data
       data.merge \
         async_id: canonical_id,

hotwire_combobox.gemspec

@@ -23,6 +23,7 @@ Gem::Specification.new do |spec|
   spec.add_dependency "rails", ">= 7.0.7.2"
   spec.add_dependency "stimulus-rails", ">= 1.2"
   spec.add_dependency "turbo-rails", ">= 1.2"
+  spec.add_dependency "platform_agent", ">= 1.0.1"
 
   spec.add_development_dependency "mocha", "~> 2.1"
 end

lib/hotwire_combobox.rb

@@ -1,5 +1,6 @@
 require "hotwire_combobox/version"
 require "hotwire_combobox/engine"
+require "hotwire_combobox/platform"
 
 module HotwireCombobox
   mattr_accessor :bypass_convenience_methods

lib/hotwire_combobox/helper.rb

@@ -16,7 +16,7 @@ def hw_combobox_style_tag(*args, **kwargs)
 
     def hw_combobox_tag(name, options_or_src = [], render_in: {}, include_blank: nil, **kwargs, &block)
       options, src = hw_extract_options_and_src options_or_src, render_in, include_blank
-      component = HotwireCombobox::Component.new self, name, options: options, async_src: src, **kwargs
+      component = HotwireCombobox::Component.new self, name, options: options, async_src: src, request: request, **kwargs
       render component, &block
     end
 

lib/hotwire_combobox/platform.rb

@@ -0,0 +1,15 @@
+require "platform_agent"
+
+class HotwireCombobox::Platform < PlatformAgent
+  def ios?
+    mobile_webkit? && !android?
+  end
+
+  def android?
+    match?(/Android/)
+  end
+
+  def mobile_webkit?
+    match?(/AppleWebKit/) && match?(/Mobile/)
+  end
+end

test/application_system_test_case.rb

@@ -1,6 +1,6 @@
 class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
   include ComboboxActionsHelper
-  include ComboboxAssertionsHelper
+  include ComboboxAssertionsHelper, ViewportAssertionsHelper
 
   driven_by :selenium, using: :headless_chrome
 

test/dummy/app/controllers/comboboxes_controller.rb

@@ -4,6 +4,9 @@ class ComboboxesController < ApplicationController
   def plain
   end
 
+  def padded
+  end
+
   def open
   end
 

test/dummy/app/views/comboboxes/padded.html.erb

@@ -0,0 +1,12 @@
+<%= tag.style nonce: content_security_policy_nonce do %>
+  .wrapper {
+    border: 2px dashed green;
+    padding-top: 2000px;
+  }
+<% end %>
+
+<h1 id="to-be-hidden-by-dialog">To be hidden by dialog</h1>
+
+<div class="wrapper">
+  <%= combobox_tag "state", @state_options, id: "state-field" %>
+</div>

test/dummy/config/routes.rb

@@ -1,5 +1,6 @@
 Rails.application.routes.draw do
   get "plain", to: "comboboxes#plain"
+  get "padded", to: "comboboxes#padded"
   get "open", to: "comboboxes#open"
   get "html_options", to: "comboboxes#html_options"
   get "prefilled", to: "comboboxes#prefilled"

test/lib/helpers/viewport_assertions_helper.rb

@@ -0,0 +1,23 @@
+module ViewportAssertionsHelper
+  def assert_in_viewport(element, message = "Expected element to be in the viewport, but it was not visible.")
+    assert element_in_viewport?(element), message
+  end
+
+  def assert_not_in_viewport(element, message = "Expected element to be outside the viewport, but it was visible.")
+    assert_not element_in_viewport?(element), message
+  end
+
+  private
+    def element_in_viewport?(element)
+      page.execute_script(<<-JS, element)
+        const el = arguments[0]
+        const rect = el.getBoundingClientRect()
+        return (
+          rect.top >= 0 &&
+          rect.left >= 0 &&
+          rect.bottom <= (window.innerHeight || document.documentElement.clientHeight) &&
+          rect.right <= (window.innerWidth || document.documentElement.clientWidth)
+        )
+      JS
+    end
+end

test/system/dialog_test.rb

@@ -22,13 +22,20 @@ class DialogTest < ApplicationSystemTestCase
     end
   end
 
-  test "selecting options within a dialog" do
-    visit dialog_path
+  test "no scrolling behind dialog" do
+    # On mobile Safari — Manually test opening combobox, selecting, then re-opening.
 
-    click_on "Show modal"
+    on_small_screen do
+      visit padded_path
+
+      title_element = find("#to-be-hidden-by-dialog")
 
-    open_combobox "#movie_rating"
-    click_on_option "R"
-    assert_combobox_display_and_value "#movie_rating", "R", Movie.ratings[:R]
+      assert_in_viewport title_element
+      page.scroll_to(find("#state-field"))
+      assert_not_in_viewport title_element
+
+      open_combobox "#state-field"
+      assert_not_in_viewport title_element
+    end
   end
 end

test/system/hotwire_combobox_test.rb

@@ -164,4 +164,14 @@ class HotwireComboboxTest < ApplicationSystemTestCase
 
     assert_combobox_display_and_value "#form_state_id", "Alabama", states(:alabama).id
   end
+
+  test "selecting options within a modal dialog" do
+    visit dialog_path
+
+    click_on "Show modal"
+
+    open_combobox "#movie_rating"
+    click_on_option "R"
+    assert_combobox_display_and_value "#movie_rating", "R", Movie.ratings[:R]
+  end
 end

test/system_test_helper.rb

@@ -11,10 +11,9 @@
 Capybara.default_max_wait_time = 10
 
 Minitest.after_run do
-  puts "\n"
-  puts <<~WARNING
-    ⚠️ Warning
-    Focus tests might pass on Selenium but not when checked manually on Chrome.
-    Make sure you grep for `assert_focused_combobox` and test manually before releasing a new version.
+  puts "\n" + <<~WARNING
+    ✋ Warning
+    * "no scrolling behind dialog" test needs to be checked manually on mobile Safari (device, not emulator).
+    * Tests using `assert_focused_combobox` need to be checked manually on Chrome
   WARNING
 end