From 8708c98ddb2f799ece3e1d546a8ec8f5357a7c53 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Mon, 18 Nov 2024 15:16:19 -0600 Subject: [PATCH] Use platform_agent to set platform classes --- Gemfile.lock | 4 ++++ .../controllers/hw_combobox_controller.js | 2 -- .../javascripts/hotwire_combobox.esm.js | 22 ------------------- .../hw_combobox/models/combobox.js | 1 - .../hw_combobox/models/combobox/devices.js | 21 ------------------ app/presenters/hotwire_combobox/component.rb | 7 +++--- .../component/markup/dialog.rb | 11 +++++++++- hotwire_combobox.gemspec | 1 + lib/hotwire_combobox.rb | 1 + lib/hotwire_combobox/helper.rb | 2 +- lib/hotwire_combobox/platform.rb | 15 +++++++++++++ test/system/dialog_test.rb | 2 ++ test/system_test_helper.rb | 9 ++++---- 13 files changed, 42 insertions(+), 56 deletions(-) delete mode 100644 app/assets/javascripts/hw_combobox/models/combobox/devices.js create mode 100644 lib/hotwire_combobox/platform.rb diff --git a/Gemfile.lock b/Gemfile.lock index 715a0831..ff4f2ca1 100644 --- a/Gemfile.lock +++ b/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) diff --git a/app/assets/javascripts/controllers/hw_combobox_controller.js b/app/assets/javascripts/controllers/hw_combobox_controller.js index 42cba8fe..4afea9e6 100644 --- a/app/assets/javascripts/controllers/hw_combobox_controller.js +++ b/app/assets/javascripts/controllers/hw_combobox_controller.js @@ -12,7 +12,6 @@ const concerns = [ Combobox.AsyncLoading, Combobox.Autocomplete, Combobox.Callbacks, - Combobox.Devices, Combobox.Dialog, Combobox.Events, Combobox.Filtering, @@ -55,7 +54,6 @@ export default class HwComboboxController extends Concerns(...concerns) { } initialize() { - this._initializeDevice() this._initializeActors() this._initializeFiltering() this._initializeCallbacks() diff --git a/app/assets/javascripts/hotwire_combobox.esm.js b/app/assets/javascripts/hotwire_combobox.esm.js index bedcc5bb..24041c7c 100644 --- a/app/assets/javascripts/hotwire_combobox.esm.js +++ b/app/assets/javascripts/hotwire_combobox.esm.js @@ -248,26 +248,6 @@ Combobox.Callbacks = Base => class extends Base { } }; -Combobox.Devices = Base => class extends Base { - _initializeDevice() { - this.element.classList.toggle("hw-combobox--ios", this._isiOS); - this.element.classList.toggle("hw-combobox--android", this._isAndroid); - this.element.classList.toggle("hw-combobox--mobile-webkit", this._isMobileWebkit); - } - - get _isiOS() { - return this._isMobileWebkit && !this._isAndroid - } - - get _isAndroid() { - return window.navigator.userAgent.includes("Android") - } - - get _isMobileWebkit() { - return window.navigator.userAgent.includes("AppleWebKit") && window.navigator.userAgent.includes("Mobile") - } -}; - Combobox.Dialog = Base => class extends Base { rerouteListboxStreamToDialog({ detail: { newStream } }) { if (newStream.target == this.listboxTarget.id && this._dialogIsOpen) { @@ -1724,7 +1704,6 @@ const concerns = [ Combobox.AsyncLoading, Combobox.Autocomplete, Combobox.Callbacks, - Combobox.Devices, Combobox.Dialog, Combobox.Events, Combobox.Filtering, @@ -1767,7 +1746,6 @@ class HwComboboxController extends Concerns(...concerns) { } initialize() { - this._initializeDevice(); this._initializeActors(); this._initializeFiltering(); this._initializeCallbacks(); diff --git a/app/assets/javascripts/hw_combobox/models/combobox.js b/app/assets/javascripts/hw_combobox/models/combobox.js index 28b76966..5d91deec 100644 --- a/app/assets/javascripts/hw_combobox/models/combobox.js +++ b/app/assets/javascripts/hw_combobox/models/combobox.js @@ -5,7 +5,6 @@ import "hw_combobox/models/combobox/announcements" import "hw_combobox/models/combobox/async_loading" import "hw_combobox/models/combobox/autocomplete" import "hw_combobox/models/combobox/callbacks" -import "hw_combobox/models/combobox/devices" import "hw_combobox/models/combobox/dialog" import "hw_combobox/models/combobox/events" import "hw_combobox/models/combobox/filtering" diff --git a/app/assets/javascripts/hw_combobox/models/combobox/devices.js b/app/assets/javascripts/hw_combobox/models/combobox/devices.js deleted file mode 100644 index c9a7c739..00000000 --- a/app/assets/javascripts/hw_combobox/models/combobox/devices.js +++ /dev/null @@ -1,21 +0,0 @@ -import Combobox from "hw_combobox/models/combobox/base" - -Combobox.Devices = Base => class extends Base { - _initializeDevice() { - this.element.classList.toggle("hw-combobox--ios", this._isiOS) - this.element.classList.toggle("hw-combobox--android", this._isAndroid) - this.element.classList.toggle("hw-combobox--mobile-webkit", this._isMobileWebkit) - } - - get _isiOS() { - return this._isMobileWebkit && !this._isAndroid - } - - get _isAndroid() { - return window.navigator.userAgent.includes("Android") - } - - get _isMobileWebkit() { - return window.navigator.userAgent.includes("AppleWebKit") && window.navigator.userAgent.includes("Mobile") - } -} diff --git a/app/presenters/hotwire_combobox/component.rb b/app/presenters/hotwire_combobox/component.rb index e0cdd97a..1a4d6143 100644 --- a/app/presenters/hotwire_combobox/component.rb +++ b/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 diff --git a/app/presenters/hotwire_combobox/component/markup/dialog.rb b/app/presenters/hotwire_combobox/component/markup/dialog.rb index cba69899..3a0a9011 100644 --- a/app/presenters/hotwire_combobox/component/markup/dialog.rb +++ b/app/presenters/hotwire_combobox/component/markup/dialog.rb @@ -5,7 +5,7 @@ def dialog_wrapper_attrs def dialog_attrs customize :dialog, base: { - class: "hw-combobox__dialog", role: :dialog, data: { + class: "hw-combobox__dialog #{platform_classes}", role: :dialog, data: { action: "keydown->hw-combobox#navigate", hw_combobox_target: "dialog" } } end @@ -34,6 +34,15 @@ def dialog_focus_trap_attrs 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 dialog_input_id "#{canonical_id}-hw-dialog-combobox" end diff --git a/hotwire_combobox.gemspec b/hotwire_combobox.gemspec index 14bc9847..6ff22365 100644 --- a/hotwire_combobox.gemspec +++ b/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 diff --git a/lib/hotwire_combobox.rb b/lib/hotwire_combobox.rb index cedf8a76..72420cde 100644 --- a/lib/hotwire_combobox.rb +++ b/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 diff --git a/lib/hotwire_combobox/helper.rb b/lib/hotwire_combobox/helper.rb index b27de8e0..592fec1a 100644 --- a/lib/hotwire_combobox/helper.rb +++ b/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 diff --git a/lib/hotwire_combobox/platform.rb b/lib/hotwire_combobox/platform.rb new file mode 100644 index 00000000..6ef34005 --- /dev/null +++ b/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 diff --git a/test/system/dialog_test.rb b/test/system/dialog_test.rb index 317f84da..2198bfbb 100644 --- a/test/system/dialog_test.rb +++ b/test/system/dialog_test.rb @@ -23,6 +23,8 @@ class DialogTest < ApplicationSystemTestCase end test "no scrolling behind dialog" do + # On mobile Safari — Manually test opening combobox, selecting, then re-opening. + on_small_screen do visit padded_path diff --git a/test/system_test_helper.rb b/test/system_test_helper.rb index ed2e8ed6..a14f3608 100644 --- a/test/system_test_helper.rb +++ b/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