Skip to content

Commit 8c7537d

Browse files
FivellFivell
authored
Bind the popup's presets without waiting a tick (#14)
* Show the popup is inert for a tick after it opens The preset handlers and the dismissal listener are attached inside `$('body').append(...).ready(fn)`. jQuery 3 rebuilt .ready() on a Deferred, whose .then schedules through setTimeout, so on a non-document collection the callback lands a macrotask late and the popup is on screen but unbound until then. Both examples drive their clicks from a single execute_script, so they land in the same tick: one clicks a preset, the other clicks outside to dismiss. The preset click hits an unbound popup and the input stays empty; the dismissal click does nothing and the popup stays up. This commit is expected to fail CI. * Bind the popup's presets without waiting a tick Nothing here waits for anything, so bind straight away instead of deferring through .ready(). Keep the node returned by appendTo rather than re-querying `.block_timerange` by class, which only ever found the right popup because unbindClickEventBlockTimerange removes every other one first. jQuery 4 keeps the same .ready() implementation, so it is not a version cliff -- just a macrotask nobody needs. (An earlier draft of this commit claimed jQuery 4 removed the method; jQuery.fn.ready is present in 4.0.0 with a byte-identical body.)
1 parent ba2dd3c commit 8c7537d

2 files changed

Lines changed: 107 additions & 64 deletions

File tree

‎spec/popup_binding_spec.rb‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
require 'spec_helper'
2+
3+
describe 'binding the preset handlers', type: :feature, js: true do
4+
5+
let(:today) { Time.now.utc.to_date }
6+
7+
before { add_post_resource }
8+
before { visit '/admin/posts' }
9+
10+
def gteq_value
11+
page.find('.filter_date_range').all('input[type="text"]').first.value
12+
end
13+
14+
# Both clicks happen in one task, so the preset is clicked in the same tick
15+
# the popup was appended. Anything deferred to the next tick is not bound
16+
# yet and the click lands on nothing.
17+
# Wait for the link through Capybara first, otherwise a page that has not
18+
# finished rendering fails as a JS TypeError instead of an assertion.
19+
before { page.find('.filter_date_range a.btn_timerange') }
20+
21+
it 'has the presets live as soon as the popup is in the document' do
22+
page.execute_script(<<~JS)
23+
var btn = document.querySelector('.filter_date_range a.btn_timerange');
24+
btn.click();
25+
document.querySelector('.block_timerange .btn_today').click();
26+
JS
27+
28+
expect(gteq_value).to eq(today.strftime('%Y-%m-%d'))
29+
end
30+
31+
it 'has the dismissal listener live just as soon' do
32+
page.execute_script(<<~JS)
33+
document.querySelector('.filter_date_range a.btn_timerange').click();
34+
document.body.click();
35+
JS
36+
37+
expect(page).to have_no_css('.block_timerange')
38+
end
39+
end

‎vendor/assets/javascripts/active_admin_date_range_preset.js‎

Lines changed: 68 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,13 @@ $(function () {
103103
return additional_items_html += '<div><span class="btn_date_range_' + i + '">' + el['title'] + '</span></div>';
104104
});
105105

106-
$('body').append('<div style="min-width: '+e.target.offsetWidth+'px; top: '+(e.target.offsetTop)+'px; left: '+(e.target.offsetLeft)+'px" class="block_timerange">' +
106+
// Bind in the same tick the popup is appended. jQuery 3 reimplemented
107+
// .ready() on top of a Deferred, whose .then schedules through
108+
// setTimeout, so handlers attached inside it landed a macrotask late
109+
// and the popup was on screen but inert until then. jQuery 4 keeps
110+
// that implementation, so waiting buys nothing there either, and
111+
// .ready() on a non-document collection has been deprecated since 3.0.
112+
let container = $('<div style="min-width: '+e.target.offsetWidth+'px; top: '+(e.target.offsetTop)+'px; left: '+(e.target.offsetLeft)+'px" class="block_timerange">' +
107113
'<div><span class="btn_today">Today</span></div>' +
108114
'<div><span class="btn_yesterday">Yesterday</span></div>' +
109115
'<div><span class="btn_week">This Week</span></div>' +
@@ -112,78 +118,76 @@ $(function () {
112118
'<div><span class="btn_last_month">Last Month</span></div>' +
113119
additional_items_html +
114120
'</div>'
115-
).ready(function() {
116-
let container = $(this).find('.block_timerange');
117-
118-
// additional ranges
119-
opts.add_range.forEach(function(el, i) {
120-
$(container).on('click.CalendarRangeSet', '.btn_date_range_' + i, function(e) {
121-
unbindClickEventBlockTimerange();
122-
let start = new Date(el['start'].getFullYear(), el['start'].getMonth(), el['start'].getDate());
123-
let end = new Date(el['end'].getFullYear(), el['end'].getMonth(), el['end'].getDate());
124-
fillInputs(start, end)
125-
});
126-
});
121+
).appendTo('body');
127122

128-
// Today
129-
$(container).on('click.CalendarRangeSet', '.btn_today', function(e) {
123+
// additional ranges
124+
opts.add_range.forEach(function(el, i) {
125+
container.on('click.CalendarRangeSet', '.btn_date_range_' + i, function(e) {
130126
unbindClickEventBlockTimerange();
131-
let start = new Date(datetime.getFullYear(), datetime.getMonth(), datetime.getDate());
132-
let end = new Date(datetime.getFullYear(), datetime.getMonth(), datetime.getDate() + 1);
133-
fillInputs(start, end);
127+
let start = new Date(el['start'].getFullYear(), el['start'].getMonth(), el['start'].getDate());
128+
let end = new Date(el['end'].getFullYear(), el['end'].getMonth(), el['end'].getDate());
129+
fillInputs(start, end)
134130
});
131+
});
135132

136-
// Yesterday
137-
$(container).on('click.CalendarRangeSet', '.btn_yesterday', function(e) {
138-
unbindClickEventBlockTimerange();
139-
let start = new Date(datetime.getFullYear(), datetime.getMonth(), datetime.getDate() - 1);
140-
let end = new Date(datetime.getFullYear(), datetime.getMonth(), datetime.getDate());
141-
fillInputs(start, end);
142-
});
133+
// Today
134+
container.on('click.CalendarRangeSet', '.btn_today', function(e) {
135+
unbindClickEventBlockTimerange();
136+
let start = new Date(datetime.getFullYear(), datetime.getMonth(), datetime.getDate());
137+
let end = new Date(datetime.getFullYear(), datetime.getMonth(), datetime.getDate() + 1);
138+
fillInputs(start, end);
139+
});
143140

144-
// Week
145-
$(container).on('click.CalendarRangeSet', '.btn_week', function(e) {
146-
unbindClickEventBlockTimerange();
147-
let start = new Date(datetime.getFullYear(), datetime.getMonth(), datetime.getDate() - days_since_monday(datetime));
148-
let end = new Date(start.getFullYear(), start.getMonth(), start.getDate() + 7);
149-
fillInputs(start, end);
150-
});
141+
// Yesterday
142+
container.on('click.CalendarRangeSet', '.btn_yesterday', function(e) {
143+
unbindClickEventBlockTimerange();
144+
let start = new Date(datetime.getFullYear(), datetime.getMonth(), datetime.getDate() - 1);
145+
let end = new Date(datetime.getFullYear(), datetime.getMonth(), datetime.getDate());
146+
fillInputs(start, end);
147+
});
151148

152-
// Month
153-
$(container).on('click.CalendarRangeSet', '.btn_month', function(e) {
154-
unbindClickEventBlockTimerange();
155-
let start = new Date(datetime.getFullYear(), datetime.getMonth(), 1);
156-
let end = new Date(datetime.getFullYear(), datetime.getMonth() + 1, 1);
157-
fillInputs(start, end);
158-
});
149+
// Week
150+
container.on('click.CalendarRangeSet', '.btn_week', function(e) {
151+
unbindClickEventBlockTimerange();
152+
let start = new Date(datetime.getFullYear(), datetime.getMonth(), datetime.getDate() - days_since_monday(datetime));
153+
let end = new Date(start.getFullYear(), start.getMonth(), start.getDate() + 7);
154+
fillInputs(start, end);
155+
});
159156

160-
// Last Week
161-
$(container).on('click.CalendarRangeSet', '.btn_last_week', function(e) {
162-
unbindClickEventBlockTimerange();
163-
let end = new Date(datetime.getFullYear(), datetime.getMonth(), datetime.getDate() - days_since_monday(datetime));
164-
let start = new Date(end.getFullYear(), end.getMonth(), end.getDate() - 7);
165-
fillInputs(start, end);
166-
});
157+
// Month
158+
container.on('click.CalendarRangeSet', '.btn_month', function(e) {
159+
unbindClickEventBlockTimerange();
160+
let start = new Date(datetime.getFullYear(), datetime.getMonth(), 1);
161+
let end = new Date(datetime.getFullYear(), datetime.getMonth() + 1, 1);
162+
fillInputs(start, end);
163+
});
167164

168-
// Last Month
169-
$(container).on('click.CalendarRangeSet', '.btn_last_month', function(e) {
170-
unbindClickEventBlockTimerange();
171-
let end = new Date(datetime.getFullYear(), datetime.getMonth(), 1);
172-
let start = new Date(end.getFullYear(), end.getMonth() - 1, 1);
173-
fillInputs(start, end);
174-
});
165+
// Last Week
166+
container.on('click.CalendarRangeSet', '.btn_last_week', function(e) {
167+
unbindClickEventBlockTimerange();
168+
let end = new Date(datetime.getFullYear(), datetime.getMonth(), datetime.getDate() - days_since_monday(datetime));
169+
let start = new Date(end.getFullYear(), end.getMonth(), end.getDate() - 7);
170+
fillInputs(start, end);
171+
});
175172

176-
// Outer
177-
// No stopPropagation here: this listener only watches for a click
178-
// that should close the popup. Active Admin delegates Clear Filters
179-
// and the has_many buttons from <document>, which is upstream of
180-
// <body>, so swallowing the event would break them for as long as
181-
// the popup is open.
182-
$('body').on('click.CalendarRangeSet', function(e) {
183-
if ($(e.target).closest('.block_timerange').length == 0) {
184-
unbindClickEventBlockTimerange();
185-
}
186-
});
173+
// Last Month
174+
container.on('click.CalendarRangeSet', '.btn_last_month', function(e) {
175+
unbindClickEventBlockTimerange();
176+
let end = new Date(datetime.getFullYear(), datetime.getMonth(), 1);
177+
let start = new Date(end.getFullYear(), end.getMonth() - 1, 1);
178+
fillInputs(start, end);
179+
});
180+
181+
// Outer
182+
// No stopPropagation here: this listener only watches for a click
183+
// that should close the popup. Active Admin delegates Clear Filters
184+
// and the has_many buttons from <document>, which is upstream of
185+
// <body>, so swallowing the event would break them for as long as
186+
// the popup is open.
187+
$('body').on('click.CalendarRangeSet', function(e) {
188+
if ($(e.target).closest('.block_timerange').length == 0) {
189+
unbindClickEventBlockTimerange();
190+
}
187191
});
188192
});
189193
});

0 commit comments

Comments
 (0)