From 177b48dd84818eb474905e1197bfe8855bf760ef Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 2 Mar 2022 08:40:31 -0500 Subject: [PATCH 01/10] Support .ts files for BaseComponent sidecar script --- app/components/base_component.rb | 2 +- spec/components/base_component_spec.rb | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/app/components/base_component.rb b/app/components/base_component.rb index 81ba0421ccb..9b445580b21 100644 --- a/app/components/base_component.rb +++ b/app/components/base_component.rb @@ -8,7 +8,7 @@ def before_render end def self.scripts - @scripts ||= _sidecar_files(['js']).map { |file| File.basename(file, '.js') } + @scripts ||= _sidecar_files(['js', 'ts']).map { |file| File.basename(file, '.*') } end def unique_id diff --git a/spec/components/base_component_spec.rb b/spec/components/base_component_spec.rb index a0f4534f118..1e980650884 100644 --- a/spec/components/base_component_spec.rb +++ b/spec/components/base_component_spec.rb @@ -27,8 +27,10 @@ def call end def self._sidecar_files(extensions) - return ['/path/to/app/components/example_component_with_script.js'] if extensions == ['js'] - super(extensions) + files = [] + files << '/components/example_component_with_script_js.js' if extensions.include?('js') + files << '/components/example_component_with_script_ts.ts' if extensions.include?('ts') + files.presence || super(extensions) end end @@ -40,7 +42,7 @@ def call it 'adds script to class variable when rendered' do expect(view_context).to receive(:enqueue_component_scripts).twice. - with('example_component_with_script') + with('example_component_with_script_js', 'example_component_with_script_ts') render_inline(ExampleComponentWithScript.new) end From c01cbd58e6db6035e1a3acabcbbf06535e1d60fc Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 2 Mar 2022 13:10:58 -0500 Subject: [PATCH 02/10] Reimplement timeout countdown as custom element **Why**: - Reduce size and scope of hot-path application pack - Avoid relying on window globals (LoginGov.countdownTimer), since they rely on fragile dependency order - Improve testability of element behavior in isolation - Allow reuse for common countdown behavior changelog: Internal, Optimziation, Reduce size of common JavaScript file --- app/components/countdown_component.html.erb | 10 ++ app/components/countdown_component.rb | 25 +++++ app/components/countdown_component.ts | 3 + app/helpers/session_timeout_warning_helper.rb | 12 +- app/javascript/app/utils/countdown-timer.js | 32 ------ app/javascript/app/utils/index.js | 8 -- app/javascript/app/utils/ms-formatter.js | 20 ---- .../packages/countdown-element/index.spec.ts | 105 ++++++++++++++++++ .../packages/countdown-element/index.ts | 82 ++++++++++++++ .../packages/countdown-element/package.json | 5 + app/javascript/packs/application.js | 1 - app/javascript/packs/session-timeout-ping.ts | 58 +++++----- .../fully_signed_in_modal_presenter.rb | 23 ++-- .../partially_signed_in_modal_presenter.rb | 23 ++-- app/views/session_timeout/_warning.html.erb | 4 +- spec/components/countdown_component_spec.rb | 52 +++++++++ .../session_timeout_warning_helper_spec.rb | 25 +++-- .../app/utils/countdown-timer_spec.js | 57 ---------- .../app/utils/ms-formatter_spec.js | 26 ----- .../fully_signed_in_modal_presenter_spec.rb | 23 +++- ...artially_signed_in_modal_presenter_spec.rb | 23 +++- 21 files changed, 397 insertions(+), 220 deletions(-) create mode 100644 app/components/countdown_component.html.erb create mode 100644 app/components/countdown_component.rb create mode 100644 app/components/countdown_component.ts delete mode 100644 app/javascript/app/utils/countdown-timer.js delete mode 100644 app/javascript/app/utils/index.js delete mode 100644 app/javascript/app/utils/ms-formatter.js create mode 100644 app/javascript/packages/countdown-element/index.spec.ts create mode 100644 app/javascript/packages/countdown-element/index.ts create mode 100644 app/javascript/packages/countdown-element/package.json create mode 100644 spec/components/countdown_component_spec.rb delete mode 100644 spec/javascripts/app/utils/countdown-timer_spec.js delete mode 100644 spec/javascripts/app/utils/ms-formatter_spec.js diff --git a/app/components/countdown_component.html.erb b/app/components/countdown_component.html.erb new file mode 100644 index 00000000000..8264d0b5f43 --- /dev/null +++ b/app/components/countdown_component.html.erb @@ -0,0 +1,10 @@ +<%= content_tag( + :'lg-countdown', + time_remaining, + **tag_options, + data: { + expiration: expiration.iso8601, + update_interval: update_interval_in_ms, + start_immediately: start_immediately, + }.merge(tag_options[:data].to_h), + ) %> diff --git a/app/components/countdown_component.rb b/app/components/countdown_component.rb new file mode 100644 index 00000000000..116fed7a9b7 --- /dev/null +++ b/app/components/countdown_component.rb @@ -0,0 +1,25 @@ +class CountdownComponent < BaseComponent + attr_reader :expiration, :update_interval, :start_immediately, :tag_options + + MILLISECONDS_PER_SECOND = 1000 + + def initialize( + expiration:, + update_interval: 1.second, + start_immediately: true, + **tag_options + ) + @expiration = expiration + @update_interval = update_interval + @start_immediately = start_immediately + @tag_options = tag_options + end + + def update_interval_in_ms + update_interval.in_seconds * MILLISECONDS_PER_SECOND + end + + def time_remaining + distance_of_time_in_words(Time.zone.now, expiration, true) + end +end diff --git a/app/components/countdown_component.ts b/app/components/countdown_component.ts new file mode 100644 index 00000000000..772a38f46fe --- /dev/null +++ b/app/components/countdown_component.ts @@ -0,0 +1,3 @@ +import { CountdownElement } from '@18f/identity-countdown-element'; + +customElements.define('lg-countdown', CountdownElement); diff --git a/app/helpers/session_timeout_warning_helper.rb b/app/helpers/session_timeout_warning_helper.rb index a2ef3f57dff..5e53d483462 100644 --- a/app/helpers/session_timeout_warning_helper.rb +++ b/app/helpers/session_timeout_warning_helper.rb @@ -11,6 +11,10 @@ def session_timeout_warning IdentityConfig.store.session_timeout_warning_seconds end + def expires_at + session[:session_expires_at]&.to_datetime || Time.zone.now - 1 + end + def timeout_refresh_path UriService.add_params( request.original_fullpath, @@ -18,15 +22,11 @@ def timeout_refresh_path )&.html_safe # rubocop:disable Rails/OutputSafety end - def time_left_in_session - distance_of_time_in_words(session_timeout_warning, 0) - end - def session_modal if user_fully_authenticated? - FullySignedInModalPresenter.new(time_left_in_session) + FullySignedInModalPresenter.new(expires_at) else - PartiallySignedInModalPresenter.new(time_left_in_session) + PartiallySignedInModalPresenter.new(expires_at) end end end diff --git a/app/javascript/app/utils/countdown-timer.js b/app/javascript/app/utils/countdown-timer.js deleted file mode 100644 index d3f96536f14..00000000000 --- a/app/javascript/app/utils/countdown-timer.js +++ /dev/null @@ -1,32 +0,0 @@ -const { msFormatter } = require('./ms-formatter'); - -export function countdownTimer(el, timeLeft = 0, endTime = null, interval = 1000) { - let remaining = timeLeft; - let currentTime; - let timer; - - if (!el || !('innerHTML' in el)) { - return; - } - - function tick() { - /* eslint-disable no-param-reassign */ - if (endTime) { - currentTime = new Date().getTime(); - remaining = endTime - currentTime; - } - - el.childNodes[0].nodeValue = msFormatter(remaining); - - if (remaining <= 0) { - clearInterval(timer); - return; - } - - remaining -= interval; - } - - tick(); - timer = setInterval(tick, interval); - return timer; -} diff --git a/app/javascript/app/utils/index.js b/app/javascript/app/utils/index.js deleted file mode 100644 index 5c4c6934f5d..00000000000 --- a/app/javascript/app/utils/index.js +++ /dev/null @@ -1,8 +0,0 @@ -import { countdownTimer } from './countdown-timer'; -import { msFormatter } from './ms-formatter'; - -window.LoginGov = window.LoginGov || {}; -const { LoginGov } = window; - -LoginGov.countdownTimer = countdownTimer; -LoginGov.msFormatter = msFormatter; diff --git a/app/javascript/app/utils/ms-formatter.js b/app/javascript/app/utils/ms-formatter.js deleted file mode 100644 index 94997943781..00000000000 --- a/app/javascript/app/utils/ms-formatter.js +++ /dev/null @@ -1,20 +0,0 @@ -import { t } from '@18f/identity-i18n'; - -// i18n-tasks-use t('datetime.dotiw.seconds') -// i18n-tasks-use t('datetime.dotiw.minutes') -const formatTime = (time, unit) => t(`datetime.dotiw.${unit}`, { count: time }); - -export function msFormatter(milliseconds) { - const seconds = milliseconds / 1000; - const minutes = parseInt(seconds / 60, 10); - const remainingSeconds = parseInt(seconds % 60, 10); - - const displayMinutes = formatTime(minutes, 'minutes'); - const displaySeconds = formatTime(remainingSeconds, 'seconds'); - - const displayTime = `${displayMinutes}${t( - 'datetime.dotiw.two_words_connector', - )}${displaySeconds}`; - - return displayTime; -} diff --git a/app/javascript/packages/countdown-element/index.spec.ts b/app/javascript/packages/countdown-element/index.spec.ts new file mode 100644 index 00000000000..519a2b65377 --- /dev/null +++ b/app/javascript/packages/countdown-element/index.spec.ts @@ -0,0 +1,105 @@ +import sinon from 'sinon'; +import { i18n } from '@18f/identity-i18n'; +import { usePropertyValue } from '@18f/identity-test-helpers'; +import { CountdownElement, CountdownElementDataset } from './index'; + +const DEFAULT_DATASET: CountdownElementDataset = { + updateInterval: '1000', + startImmediately: 'true', + expiration: new Date().toISOString(), +}; + +describe('CountdownElement', () => { + let clock: sinon.SinonFakeTimers; + + usePropertyValue(i18n, 'strings', { + 'datetime.dotiw.seconds': { one: 'one second', other: '%{count} seconds' }, + 'datetime.dotiw.minutes': { one: 'one minute', other: '%{count} minutes' }, + 'datetime.dotiw.two_words_connector': ' and ', + }); + + before(() => { + if (!customElements.get('lg-countdown')) { + customElements.define('lg-countdown', CountdownElement); + } + + clock = sinon.useFakeTimers(); + }); + + after(() => { + clock.restore(); + }); + + function createElement(dataset: Partial = {}) { + const element = document.createElement('lg-countdown') as CountdownElement; + Object.assign(element.dataset, DEFAULT_DATASET, dataset); + document.body.appendChild(element); + return element; + } + + it('sets text to formatted date', () => { + const element = createElement({ + expiration: new Date(new Date().getTime() + 62000).toISOString(), + }); + + expect(element.textContent).to.equal('one minute and 2 seconds'); + }); + + it('schedules update after interval', () => { + const element = createElement({ + expiration: new Date(new Date().getTime() + 3000).toISOString(), + updateInterval: '2000', + }); + + clock.tick(1999); + + expect(element.textContent).to.equal('0 minutes and 3 seconds'); + + clock.tick(1); + + expect(element.textContent).to.equal('0 minutes and one second'); + }); + + it('allows a delayed start', () => { + const element = createElement({ + expiration: new Date(new Date().getTime() + 1000).toISOString(), + startImmediately: 'false', + }); + + clock.tick(1000); + + expect(element.textContent).to.equal('0 minutes and one second'); + + element.start(); + + expect(element.textContent).to.equal('0 minutes and 0 seconds'); + }); + + it('can be stopped and restarted', () => { + const element = createElement({ + expiration: new Date(new Date().getTime() + 2000).toISOString(), + updateInterval: '1000', + }); + + element.stop(); + clock.tick(1000); + + expect(element.textContent).to.equal('0 minutes and 2 seconds'); + + element.start(); + + expect(element.textContent).to.equal('0 minutes and one second'); + }); + + it('updates in response to changed expiration', () => { + const element = createElement(); + + element.expiration = new Date(new Date().getTime() + 1000); + + expect(element.textContent).to.equal('0 minutes and one second'); + + element.setAttribute('data-expiration', new Date(new Date().getTime() + 2000).toISOString()); + + expect(element.textContent).to.equal('0 minutes and 2 seconds'); + }); +}); diff --git a/app/javascript/packages/countdown-element/index.ts b/app/javascript/packages/countdown-element/index.ts new file mode 100644 index 00000000000..29eea15ff1a --- /dev/null +++ b/app/javascript/packages/countdown-element/index.ts @@ -0,0 +1,82 @@ +import { t } from '@18f/identity-i18n'; + +export interface CountdownElementDataset { + /** + * ISO8601-formatted date string for countdown expiration time. + */ + expiration: string; + + /** + * Interval at which text is updated, in milliseconds. + */ + updateInterval: string; + + /** + * Whether to start the countdown as soon as the element is connected. + */ + startImmediately: 'true' | 'false'; +} + +export class CountdownElement extends HTMLElement { + #pollIntervalId?: number; + + dataset: CountdownElementDataset & DOMStringMap; + + static get observedAttributes() { + return ['data-expiration']; + } + + connectedCallback() { + if (this.startImmediately) { + this.start(); + } else { + this.setTimeRemaining(); + } + } + + disconnectedCallback() { + this.stop(); + } + + attributeChangedCallback() { + this.setTimeRemaining(); + } + + get expiration(): Date { + return new Date(this.dataset.expiration); + } + + set expiration(expiration: Date) { + this.setAttribute('data-expiration', expiration.toISOString()); + } + + get timeRemaining(): number { + return Math.max(this.expiration.getTime() - Date.now(), 0); + } + + get updateInterval(): number { + return Number(this.dataset.updateInterval); + } + + get startImmediately(): boolean { + return this.dataset.startImmediately === 'true'; + } + + start(): void { + this.setTimeRemaining(); + this.#pollIntervalId = window.setInterval(() => this.setTimeRemaining(), this.updateInterval); + } + + stop(): void { + window.clearInterval(this.#pollIntervalId); + } + + setTimeRemaining(): void { + const { timeRemaining } = this; + + this.textContent = [ + t('datetime.dotiw.minutes', { count: Math.floor(timeRemaining / 60000) }), + t('datetime.dotiw.seconds', { count: Math.floor(timeRemaining / 1000) % 60 }), + ].join(t('datetime.dotiw.two_words_connector')); + } +} diff --git a/app/javascript/packages/countdown-element/package.json b/app/javascript/packages/countdown-element/package.json new file mode 100644 index 00000000000..137e7a52a57 --- /dev/null +++ b/app/javascript/packages/countdown-element/package.json @@ -0,0 +1,5 @@ +{ + "name": "@18f/identity-countdown-element", + "private": true, + "version": "1.0.0" +} diff --git a/app/javascript/packs/application.js b/app/javascript/packs/application.js index 802a1112228..68354759319 100644 --- a/app/javascript/packs/application.js +++ b/app/javascript/packs/application.js @@ -1,5 +1,4 @@ require('../app/components/index'); -require('../app/utils/index'); require('../app/pw-toggle'); require('../app/print-personal-key'); require('../app/i18n-dropdown'); diff --git a/app/javascript/packs/session-timeout-ping.ts b/app/javascript/packs/session-timeout-ping.ts index b4182eae7e6..c077ebe7b46 100644 --- a/app/javascript/packs/session-timeout-ping.ts +++ b/app/javascript/packs/session-timeout-ping.ts @@ -1,3 +1,5 @@ +import type { CountdownElement } from '@18f/identity-countdown-element'; + interface NewRelicAgent { /** * Log page action to New Relic. @@ -7,14 +9,6 @@ interface NewRelicAgent { interface LoginGov { Modal: (any) => void; - - countdownTimer: ( - el: HTMLElement | null, - timeLeft: number, - endTime: number, - interval?: number, - screenReader?: boolean, - ) => void; } interface NewRelicGlobals { @@ -28,6 +22,23 @@ interface LoginGovGlobals { LoginGov: LoginGov; } +interface PingResponse { + /** + * Whether the session is still active. + */ + live: boolean; + + /** + * Time remaining in active session, in seconds. + */ + remaining: number; + + /** + * ISO8601-formatted date string for session timeout. + */ + timeout: string; +} + type LoginGovGlobal = typeof window & NewRelicGlobals & LoginGovGlobals; const login = (window as LoginGovGlobal).LoginGov; @@ -35,7 +46,6 @@ const login = (window as LoginGovGlobal).LoginGov; const warningEl = document.getElementById('session-timeout-cntnr'); const defaultTime = '60'; -const SR_MESSAGE_UPDATE_INTERVAL_SECONDS = 30; const frequency = parseInt(warningEl?.dataset.frequency || defaultTime, 10) * 1000; const warning = parseInt(warningEl?.dataset.warning || defaultTime, 10) * 1000; @@ -47,6 +57,7 @@ const initialTime = new Date(); const modal = new login.Modal({ el: '#session-timeout-msg' }); const keepaliveEl = document.getElementById('session-keepalive-btn'); +const countdownEls: NodeListOf = modal.el.querySelectorAll('lg-countdown'); const csrfEl: HTMLMetaElement | null = document.querySelector('meta[name="csrf-token"]'); let csrfToken = ''; @@ -54,9 +65,6 @@ if (csrfEl) { csrfToken = csrfEl.content; } -let countdownInterval; -let srCountdownInterval; - function notifyNewRelic(request, error, actionName) { (window as LoginGovGlobal).newrelic?.addPageAction('Session Ping Error', { action_name: actionName, @@ -72,9 +80,8 @@ function forceRedirect(redirectURL: string) { window.location.href = redirectURL; } -function success(data) { +function success(data: PingResponse) { let timeRemaining = data.remaining * 1000; - const timeTimeout = new Date().getTime() + timeRemaining; const showWarning = timeRemaining < warning; if (!data.live) { @@ -86,28 +93,15 @@ function success(data) { if (showWarning && !modal.shown) { modal.show(); - - if (countdownInterval) { - clearInterval(countdownInterval); - } - countdownInterval = login.countdownTimer( - document.getElementById('countdown'), - timeRemaining, - timeTimeout, - ); - if (srCountdownInterval) { - clearInterval(srCountdownInterval); - } - srCountdownInterval = login.countdownTimer( - document.getElementById('sr-countdown'), - timeRemaining, - timeTimeout, - SR_MESSAGE_UPDATE_INTERVAL_SECONDS * 1000, - ); + countdownEls.forEach((countdownEl) => { + countdownEl.expiration = new Date(data.timeout); + countdownEl.start(); + }); } if (!showWarning && modal.shown) { modal.hide(); + countdownEls.forEach((countdownEl) => countdownEl.stop()); } if (timeRemaining < frequency) { diff --git a/app/presenters/fully_signed_in_modal_presenter.rb b/app/presenters/fully_signed_in_modal_presenter.rb index 4a7dc4c9805..b20c64833f0 100644 --- a/app/presenters/fully_signed_in_modal_presenter.rb +++ b/app/presenters/fully_signed_in_modal_presenter.rb @@ -1,22 +1,29 @@ class FullySignedInModalPresenter - include ActionView::Helpers::TagHelper include ActionView::Helpers::TranslationHelper - def initialize(time_left_in_session) - @time_left_in_session = time_left_in_session + def initialize(expiration) + @expiration = expiration end - def message + def message(view_context) t( 'notices.timeout_warning.signed_in.message_html', - time_left_in_session: content_tag(:span, time_left_in_session, id: 'countdown'), + time_left_in_session: view_context.render_to_string( + CountdownComponent.new(expiration: expiration, start_immediately: false), + ), ) end - def sr_message + def sr_message(view_context) t( 'notices.timeout_warning.signed_in.sr_message_html', - time_left_in_session: content_tag(:span, time_left_in_session, id: 'sr-countdown'), + time_left_in_session: view_context.render_to_string( + CountdownComponent.new( + expiration: expiration, + update_interval: 30.seconds, + start_immediately: false, + ), + ), ) end @@ -30,5 +37,5 @@ def sign_out private - attr_reader :time_left_in_session + attr_reader :expiration end diff --git a/app/presenters/partially_signed_in_modal_presenter.rb b/app/presenters/partially_signed_in_modal_presenter.rb index 49652c7a337..cd38e8924cd 100644 --- a/app/presenters/partially_signed_in_modal_presenter.rb +++ b/app/presenters/partially_signed_in_modal_presenter.rb @@ -1,22 +1,29 @@ class PartiallySignedInModalPresenter - include ActionView::Helpers::TagHelper include ActionView::Helpers::TranslationHelper - def initialize(time_left_in_session) - @time_left_in_session = time_left_in_session + def initialize(expiration) + @expiration = expiration end - def message + def message(view_context) t( 'notices.timeout_warning.partially_signed_in.message_html', - time_left_in_session: content_tag(:span, time_left_in_session, id: 'countdown'), + time_left_in_session: view_context.render_to_string( + CountdownComponent.new(expiration: expiration, start_immediately: false), + ), ) end - def sr_message + def sr_message(view_context) t( 'notices.timeout_warning.partially_signed_in.sr_message_html', - time_left_in_session: content_tag(:span, time_left_in_session, id: 'sr-countdown'), + time_left_in_session: view_context.render_to_string( + CountdownComponent.new( + expiration: expiration, + update_interval: 30.seconds, + start_immediately: false, + ), + ), ) end @@ -30,5 +37,5 @@ def sign_out private - attr_reader :time_left_in_session + attr_reader :expiration end diff --git a/app/views/session_timeout/_warning.html.erb b/app/views/session_timeout/_warning.html.erb index 7a5a18adb4c..6aefa53fba8 100644 --- a/app/views/session_timeout/_warning.html.erb +++ b/app/views/session_timeout/_warning.html.erb @@ -7,10 +7,10 @@

- <%= modal.message %> + <%= modal.message(self) %>

- <%= modal.sr_message %> + <%= modal.sr_message(self) %>

<%= button_tag modal.continue, diff --git a/spec/components/countdown_component_spec.rb b/spec/components/countdown_component_spec.rb new file mode 100644 index 00000000000..abfae7bf112 --- /dev/null +++ b/spec/components/countdown_component_spec.rb @@ -0,0 +1,52 @@ +require 'rails_helper' + +RSpec.describe CountdownComponent, type: :component do + let(:expiration) { Time.zone.now + 1.minute + 1.second } + + around do |ex| + freeze_time { ex.run } + end + + it 'renders element with expected attributes and initial expiration time' do + rendered = render_inline CountdownComponent.new(expiration: expiration) + + element = rendered.css('lg-countdown', text: '1 minute and 1 second').first + expect(element).to be_present + expect(element.attr('data-expiration')).to eq(expiration.iso8601) + expect(element.attr('data-update-interval')).to eq('1000') + expect(element.attr('data-start-immediately')).to eq('true') + end + + context 'with tag options' do + it 'renders with attributes' do + rendered = render_inline CountdownComponent.new( + expiration: expiration, + data: { foo: 'bar' }, + ) + + expect(rendered).to have_css('lg-countdown[data-expiration][data-foo="bar"]') + end + end + + context 'with custom update interval' do + it 'assigns update interval in milliseconds' do + rendered = render_inline CountdownComponent.new( + expiration: expiration, + update_interval: 30.seconds, + ) + + expect(rendered).to have_css('lg-countdown[data-update-interval="30000"]') + end + end + + context 'with controlled start immediately' do + it 'assigns attribute to start immediately' do + rendered = render_inline CountdownComponent.new( + expiration: expiration, + start_immediately: false, + ) + + expect(rendered).to have_css('lg-countdown[data-start-immediately="false"]') + end + end +end diff --git a/spec/helpers/session_timeout_warning_helper_spec.rb b/spec/helpers/session_timeout_warning_helper_spec.rb index ea352aa5433..f2500accc2d 100644 --- a/spec/helpers/session_timeout_warning_helper_spec.rb +++ b/spec/helpers/session_timeout_warning_helper_spec.rb @@ -1,14 +1,23 @@ require 'rails_helper' describe SessionTimeoutWarningHelper do - describe '#time_left_in_session' do - it 'describes time left based on when the timeout warning appears' do - allow(IdentityConfig.store).to receive(:session_check_frequency).and_return(1) - allow(IdentityConfig.store).to receive(:session_check_delay).and_return(2) - allow(IdentityConfig.store).to receive(:session_timeout_warning_seconds).and_return(3) - - expect(helper.time_left_in_session). - to eq distance_of_time_in_words(time_between_warning_and_timeout) + describe '#expires_at' do + around do |ex| + freeze_time { ex.run } + end + + it 'returns time before now' do + expect(helper.expires_at).to be < Time.zone.now + end + + context 'with session expiration' do + before do + allow(helper).to receive(:session).and_return(session_expires_at: Time.zone.now + 1) + end + + it 'returns time remaining in user session' do + expect(helper.expires_at).to be > Time.zone.now + end end end diff --git a/spec/javascripts/app/utils/countdown-timer_spec.js b/spec/javascripts/app/utils/countdown-timer_spec.js deleted file mode 100644 index 71d0cb39696..00000000000 --- a/spec/javascripts/app/utils/countdown-timer_spec.js +++ /dev/null @@ -1,57 +0,0 @@ -import sinon from 'sinon'; -import { i18n } from '@18f/identity-i18n'; -import { usePropertyValue } from '@18f/identity-test-helpers'; -import { countdownTimer } from '../../../../app/javascript/app/utils/countdown-timer'; - -describe('countdownTimer', () => { - it('does nothing if a HTMLElement is not supplied as the first argument', () => { - expect(countdownTimer(false)).to.be.undefined(); - }); - - describe('with clock', () => { - usePropertyValue(i18n, 'strings', { - 'datetime.dotiw.seconds': { one: 'one second', other: '%{count} seconds' }, - 'datetime.dotiw.minutes': { one: 'one minute', other: '%{count} minutes' }, - 'datetime.dotiw.two_words_connector': ' and ', - }); - - let clock; - let el; - - beforeEach(() => { - clock = sinon.useFakeTimers(); - el = document.createElement('div'); - el.appendChild(document.createTextNode('test')); - }); - - afterEach(() => { - clock.restore(); - }); - - it('stays at 0s when time is exhausted', () => { - countdownTimer(el); - - expect(el.innerHTML).to.equal('0 minutes and 0 seconds'); - clock.tick(1000); - expect(el.innerHTML).to.equal('0 minutes and 0 seconds'); - }); - - it('updates once per second', () => { - countdownTimer(el, 10000); - - expect(el.innerHTML).to.equal('0 minutes and 10 seconds'); - clock.tick(1000); - - expect(el.innerHTML).to.equal('0 minutes and 9 seconds'); - clock.tick(1000); - - expect(el.innerHTML).to.equal('0 minutes and 8 seconds'); - clock.tick(1000); - - expect(el.innerHTML).to.equal('0 minutes and 7 seconds'); - clock.tick(1000); - - expect(el.innerHTML).to.equal('0 minutes and 6 seconds'); - }); - }); -}); diff --git a/spec/javascripts/app/utils/ms-formatter_spec.js b/spec/javascripts/app/utils/ms-formatter_spec.js deleted file mode 100644 index 4bbe621c09a..00000000000 --- a/spec/javascripts/app/utils/ms-formatter_spec.js +++ /dev/null @@ -1,26 +0,0 @@ -import { i18n } from '@18f/identity-i18n'; -import { usePropertyValue } from '@18f/identity-test-helpers'; -import { msFormatter } from '../../../../app/javascript/app/utils/ms-formatter'; - -describe('#msFormatter', () => { - usePropertyValue(i18n, 'strings', { - 'datetime.dotiw.seconds': { one: 'one second', other: '%{count} seconds' }, - 'datetime.dotiw.minutes': { one: 'one minute', other: '%{count} minutes' }, - 'datetime.dotiw.two_words_connector': ' and ', - }); - - it('formats milliseconds as 0 minutes and 0 seconds)', () => { - const output = msFormatter(0); - expect(output).to.equal('0 minutes and 0 seconds'); - }); - - it('adds a leading zero if seconds are fewer than 10', () => { - const output = msFormatter(6000); - expect(output).to.equal('0 minutes and 6 seconds'); - }); - - it('adds a leading zero to minutes if minutes are fewer than 10 but greater than 0', () => { - const output = msFormatter(300000); - expect(output).to.equal('5 minutes and 0 seconds'); - }); -}); diff --git a/spec/presenters/fully_signed_in_modal_presenter_spec.rb b/spec/presenters/fully_signed_in_modal_presenter_spec.rb index a67e925374f..266ebd241c3 100644 --- a/spec/presenters/fully_signed_in_modal_presenter_spec.rb +++ b/spec/presenters/fully_signed_in_modal_presenter_spec.rb @@ -1,19 +1,30 @@ require 'rails_helper' describe FullySignedInModalPresenter do - include ActionView::Helpers::TagHelper + include ActionView::Helpers::SanitizeHelper - let(:time_left_in_session) { 10 } - subject(:presenter) { FullySignedInModalPresenter.new(time_left_in_session) } + let(:expiration) { Time.zone.now + 1.minute + 1.second } + subject(:presenter) { FullySignedInModalPresenter.new(expiration) } + + around do |ex| + freeze_time { ex.run } + end describe '#message' do it 'returns the fully signed in message' do - message = t( + expect(strip_tags(presenter.message(ActionController::Base.new))).to eq t( 'notices.timeout_warning.signed_in.message_html', - time_left_in_session: content_tag(:span, time_left_in_session, id: 'countdown'), + time_left_in_session: "1 minute and 1 second\n", ) + end + end - expect(presenter.message).to eq message + describe '#sr_message' do + it 'returns the fully signed in message for screen readers' do + expect(strip_tags(presenter.sr_message(ActionController::Base.new))).to eq t( + 'notices.timeout_warning.signed_in.sr_message_html', + time_left_in_session: "1 minute and 1 second\n", + ) end end diff --git a/spec/presenters/partially_signed_in_modal_presenter_spec.rb b/spec/presenters/partially_signed_in_modal_presenter_spec.rb index ea2aeb131a1..15217c60bb6 100644 --- a/spec/presenters/partially_signed_in_modal_presenter_spec.rb +++ b/spec/presenters/partially_signed_in_modal_presenter_spec.rb @@ -1,19 +1,30 @@ require 'rails_helper' describe PartiallySignedInModalPresenter do - include ActionView::Helpers::TagHelper + include ActionView::Helpers::SanitizeHelper - let(:time_left_in_session) { 10 } - subject(:presenter) { PartiallySignedInModalPresenter.new(time_left_in_session) } + let(:expiration) { Time.zone.now + 1.minute + 1.second } + subject(:presenter) { PartiallySignedInModalPresenter.new(expiration) } + + around do |ex| + freeze_time { ex.run } + end describe '#message' do it 'returns the partially signed in message' do - message = t( + expect(strip_tags(presenter.message(ActionController::Base.new))).to eq t( 'notices.timeout_warning.partially_signed_in.message_html', - time_left_in_session: content_tag(:span, time_left_in_session, id: 'countdown'), + time_left_in_session: "1 minute and 1 second\n", ) + end + end - expect(presenter.message).to eq message + describe '#sr_message' do + it 'returns the partially signed in message for screen readers' do + expect(strip_tags(presenter.sr_message(ActionController::Base.new))).to eq t( + 'notices.timeout_warning.partially_signed_in.sr_message_html', + time_left_in_session: "1 minute and 1 second\n", + ) end end From d25a961ad00c8a806f1bfea39028fcced2efa57c Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 14 Mar 2022 10:55:44 -0400 Subject: [PATCH 03/10] Initialize modal presenter with view_context Avoid passing from view See: https://github.com/18F/identity-idp/pull/6023#discussion_r817995755 --- app/helpers/session_timeout_warning_helper.rb | 4 ++-- app/presenters/fully_signed_in_modal_presenter.rb | 9 +++++---- app/presenters/partially_signed_in_modal_presenter.rb | 9 +++++---- app/views/session_timeout/_warning.html.erb | 4 ++-- spec/presenters/fully_signed_in_modal_presenter_spec.rb | 9 ++++++--- .../partially_signed_in_modal_presenter_spec.rb | 9 ++++++--- 6 files changed, 26 insertions(+), 18 deletions(-) diff --git a/app/helpers/session_timeout_warning_helper.rb b/app/helpers/session_timeout_warning_helper.rb index 5e53d483462..4a974aade9b 100644 --- a/app/helpers/session_timeout_warning_helper.rb +++ b/app/helpers/session_timeout_warning_helper.rb @@ -24,9 +24,9 @@ def timeout_refresh_path def session_modal if user_fully_authenticated? - FullySignedInModalPresenter.new(expires_at) + FullySignedInModalPresenter.new(view_context: self, expiration: expires_at) else - PartiallySignedInModalPresenter.new(expires_at) + PartiallySignedInModalPresenter.new(view_context: self, expiration: expires_at) end end end diff --git a/app/presenters/fully_signed_in_modal_presenter.rb b/app/presenters/fully_signed_in_modal_presenter.rb index b20c64833f0..360dc28beac 100644 --- a/app/presenters/fully_signed_in_modal_presenter.rb +++ b/app/presenters/fully_signed_in_modal_presenter.rb @@ -1,11 +1,12 @@ class FullySignedInModalPresenter include ActionView::Helpers::TranslationHelper - def initialize(expiration) + def initialize(view_context:, expiration:) + @view_context = view_context @expiration = expiration end - def message(view_context) + def message t( 'notices.timeout_warning.signed_in.message_html', time_left_in_session: view_context.render_to_string( @@ -14,7 +15,7 @@ def message(view_context) ) end - def sr_message(view_context) + def sr_message t( 'notices.timeout_warning.signed_in.sr_message_html', time_left_in_session: view_context.render_to_string( @@ -37,5 +38,5 @@ def sign_out private - attr_reader :expiration + attr_reader :expiration, :view_context end diff --git a/app/presenters/partially_signed_in_modal_presenter.rb b/app/presenters/partially_signed_in_modal_presenter.rb index cd38e8924cd..dff0009f773 100644 --- a/app/presenters/partially_signed_in_modal_presenter.rb +++ b/app/presenters/partially_signed_in_modal_presenter.rb @@ -1,11 +1,12 @@ class PartiallySignedInModalPresenter include ActionView::Helpers::TranslationHelper - def initialize(expiration) + def initialize(view_context:, expiration:) + @view_context = view_context @expiration = expiration end - def message(view_context) + def message t( 'notices.timeout_warning.partially_signed_in.message_html', time_left_in_session: view_context.render_to_string( @@ -14,7 +15,7 @@ def message(view_context) ) end - def sr_message(view_context) + def sr_message t( 'notices.timeout_warning.partially_signed_in.sr_message_html', time_left_in_session: view_context.render_to_string( @@ -37,5 +38,5 @@ def sign_out private - attr_reader :expiration + attr_reader :expiration, :view_context end diff --git a/app/views/session_timeout/_warning.html.erb b/app/views/session_timeout/_warning.html.erb index 6aefa53fba8..7a5a18adb4c 100644 --- a/app/views/session_timeout/_warning.html.erb +++ b/app/views/session_timeout/_warning.html.erb @@ -7,10 +7,10 @@

- <%= modal.message(self) %> + <%= modal.message %>

- <%= modal.sr_message(self) %> + <%= modal.sr_message %>

<%= button_tag modal.continue, diff --git a/spec/presenters/fully_signed_in_modal_presenter_spec.rb b/spec/presenters/fully_signed_in_modal_presenter_spec.rb index 266ebd241c3..417397f9bfc 100644 --- a/spec/presenters/fully_signed_in_modal_presenter_spec.rb +++ b/spec/presenters/fully_signed_in_modal_presenter_spec.rb @@ -4,7 +4,10 @@ include ActionView::Helpers::SanitizeHelper let(:expiration) { Time.zone.now + 1.minute + 1.second } - subject(:presenter) { FullySignedInModalPresenter.new(expiration) } + let(:view_context) { ActionController::Base.new } + subject(:presenter) do + FullySignedInModalPresenter.new(view_context: view_context, expiration: expiration) + end around do |ex| freeze_time { ex.run } @@ -12,7 +15,7 @@ describe '#message' do it 'returns the fully signed in message' do - expect(strip_tags(presenter.message(ActionController::Base.new))).to eq t( + expect(strip_tags(presenter.message)).to eq t( 'notices.timeout_warning.signed_in.message_html', time_left_in_session: "1 minute and 1 second\n", ) @@ -21,7 +24,7 @@ describe '#sr_message' do it 'returns the fully signed in message for screen readers' do - expect(strip_tags(presenter.sr_message(ActionController::Base.new))).to eq t( + expect(strip_tags(presenter.sr_message)).to eq t( 'notices.timeout_warning.signed_in.sr_message_html', time_left_in_session: "1 minute and 1 second\n", ) diff --git a/spec/presenters/partially_signed_in_modal_presenter_spec.rb b/spec/presenters/partially_signed_in_modal_presenter_spec.rb index 15217c60bb6..d36672298a2 100644 --- a/spec/presenters/partially_signed_in_modal_presenter_spec.rb +++ b/spec/presenters/partially_signed_in_modal_presenter_spec.rb @@ -4,7 +4,10 @@ include ActionView::Helpers::SanitizeHelper let(:expiration) { Time.zone.now + 1.minute + 1.second } - subject(:presenter) { PartiallySignedInModalPresenter.new(expiration) } + let(:view_context) { ActionController::Base.new } + subject(:presenter) do + PartiallySignedInModalPresenter.new(view_context: view_context, expiration: expiration) + end around do |ex| freeze_time { ex.run } @@ -12,7 +15,7 @@ describe '#message' do it 'returns the partially signed in message' do - expect(strip_tags(presenter.message(ActionController::Base.new))).to eq t( + expect(strip_tags(presenter.message)).to eq t( 'notices.timeout_warning.partially_signed_in.message_html', time_left_in_session: "1 minute and 1 second\n", ) @@ -21,7 +24,7 @@ describe '#sr_message' do it 'returns the partially signed in message for screen readers' do - expect(strip_tags(presenter.sr_message(ActionController::Base.new))).to eq t( + expect(strip_tags(presenter.sr_message)).to eq t( 'notices.timeout_warning.partially_signed_in.sr_message_html', time_left_in_session: "1 minute and 1 second\n", ) From cef5cec2a05e823f87d06f9b3efc43c3a0b845ea Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 14 Mar 2022 11:02:17 -0400 Subject: [PATCH 04/10] Move countdown tag contructor to call - Simplify tests to avoid accounting for newline - Because it's single, simple tag See: https://github.com/18F/identity-idp/pull/6023#discussion_r817996662 --- app/components/countdown_component.html.erb | 10 ---------- app/components/countdown_component.rb | 13 +++++++++++++ .../fully_signed_in_modal_presenter_spec.rb | 4 ++-- .../partially_signed_in_modal_presenter_spec.rb | 4 ++-- 4 files changed, 17 insertions(+), 14 deletions(-) delete mode 100644 app/components/countdown_component.html.erb diff --git a/app/components/countdown_component.html.erb b/app/components/countdown_component.html.erb deleted file mode 100644 index 8264d0b5f43..00000000000 --- a/app/components/countdown_component.html.erb +++ /dev/null @@ -1,10 +0,0 @@ -<%= content_tag( - :'lg-countdown', - time_remaining, - **tag_options, - data: { - expiration: expiration.iso8601, - update_interval: update_interval_in_ms, - start_immediately: start_immediately, - }.merge(tag_options[:data].to_h), - ) %> diff --git a/app/components/countdown_component.rb b/app/components/countdown_component.rb index 116fed7a9b7..faa15b32759 100644 --- a/app/components/countdown_component.rb +++ b/app/components/countdown_component.rb @@ -15,6 +15,19 @@ def initialize( @tag_options = tag_options end + def call + content_tag( + :'lg-countdown', + time_remaining, + **tag_options, + data: { + expiration: expiration.iso8601, + update_interval: update_interval_in_ms, + start_immediately: start_immediately, + }.merge(tag_options[:data].to_h), + ) + end + def update_interval_in_ms update_interval.in_seconds * MILLISECONDS_PER_SECOND end diff --git a/spec/presenters/fully_signed_in_modal_presenter_spec.rb b/spec/presenters/fully_signed_in_modal_presenter_spec.rb index 417397f9bfc..40a6e30110a 100644 --- a/spec/presenters/fully_signed_in_modal_presenter_spec.rb +++ b/spec/presenters/fully_signed_in_modal_presenter_spec.rb @@ -17,7 +17,7 @@ it 'returns the fully signed in message' do expect(strip_tags(presenter.message)).to eq t( 'notices.timeout_warning.signed_in.message_html', - time_left_in_session: "1 minute and 1 second\n", + time_left_in_session: '1 minute and 1 second', ) end end @@ -26,7 +26,7 @@ it 'returns the fully signed in message for screen readers' do expect(strip_tags(presenter.sr_message)).to eq t( 'notices.timeout_warning.signed_in.sr_message_html', - time_left_in_session: "1 minute and 1 second\n", + time_left_in_session: '1 minute and 1 second', ) end end diff --git a/spec/presenters/partially_signed_in_modal_presenter_spec.rb b/spec/presenters/partially_signed_in_modal_presenter_spec.rb index d36672298a2..0c2c4479d7e 100644 --- a/spec/presenters/partially_signed_in_modal_presenter_spec.rb +++ b/spec/presenters/partially_signed_in_modal_presenter_spec.rb @@ -17,7 +17,7 @@ it 'returns the partially signed in message' do expect(strip_tags(presenter.message)).to eq t( 'notices.timeout_warning.partially_signed_in.message_html', - time_left_in_session: "1 minute and 1 second\n", + time_left_in_session: '1 minute and 1 second', ) end end @@ -26,7 +26,7 @@ it 'returns the partially signed in message for screen readers' do expect(strip_tags(presenter.sr_message)).to eq t( 'notices.timeout_warning.partially_signed_in.sr_message_html', - time_left_in_session: "1 minute and 1 second\n", + time_left_in_session: '1 minute and 1 second', ) end end From 237fcc40bf0803b2fc7aa501601109fcfc9614c5 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 14 Mar 2022 11:13:54 -0400 Subject: [PATCH 05/10] Render component using view_context.render **Why**: Avoid errors in runtime --- app/presenters/fully_signed_in_modal_presenter.rb | 4 ++-- app/presenters/partially_signed_in_modal_presenter.rb | 4 ++-- spec/presenters/fully_signed_in_modal_presenter_spec.rb | 3 ++- spec/presenters/partially_signed_in_modal_presenter_spec.rb | 3 ++- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/presenters/fully_signed_in_modal_presenter.rb b/app/presenters/fully_signed_in_modal_presenter.rb index 360dc28beac..3ac8feea939 100644 --- a/app/presenters/fully_signed_in_modal_presenter.rb +++ b/app/presenters/fully_signed_in_modal_presenter.rb @@ -9,7 +9,7 @@ def initialize(view_context:, expiration:) def message t( 'notices.timeout_warning.signed_in.message_html', - time_left_in_session: view_context.render_to_string( + time_left_in_session: view_context.render( CountdownComponent.new(expiration: expiration, start_immediately: false), ), ) @@ -18,7 +18,7 @@ def message def sr_message t( 'notices.timeout_warning.signed_in.sr_message_html', - time_left_in_session: view_context.render_to_string( + time_left_in_session: view_context.render( CountdownComponent.new( expiration: expiration, update_interval: 30.seconds, diff --git a/app/presenters/partially_signed_in_modal_presenter.rb b/app/presenters/partially_signed_in_modal_presenter.rb index dff0009f773..655d87b7811 100644 --- a/app/presenters/partially_signed_in_modal_presenter.rb +++ b/app/presenters/partially_signed_in_modal_presenter.rb @@ -9,7 +9,7 @@ def initialize(view_context:, expiration:) def message t( 'notices.timeout_warning.partially_signed_in.message_html', - time_left_in_session: view_context.render_to_string( + time_left_in_session: view_context.render( CountdownComponent.new(expiration: expiration, start_immediately: false), ), ) @@ -18,7 +18,7 @@ def message def sr_message t( 'notices.timeout_warning.partially_signed_in.sr_message_html', - time_left_in_session: view_context.render_to_string( + time_left_in_session: view_context.render( CountdownComponent.new( expiration: expiration, update_interval: 30.seconds, diff --git a/spec/presenters/fully_signed_in_modal_presenter_spec.rb b/spec/presenters/fully_signed_in_modal_presenter_spec.rb index 40a6e30110a..12f229edb56 100644 --- a/spec/presenters/fully_signed_in_modal_presenter_spec.rb +++ b/spec/presenters/fully_signed_in_modal_presenter_spec.rb @@ -4,7 +4,8 @@ include ActionView::Helpers::SanitizeHelper let(:expiration) { Time.zone.now + 1.minute + 1.second } - let(:view_context) { ActionController::Base.new } + let(:lookup_context) { ActionView::LookupContext.new(ActionController::Base.view_paths) } + let(:view_context) { ActionView::Base.new(lookup_context, {}, nil) } subject(:presenter) do FullySignedInModalPresenter.new(view_context: view_context, expiration: expiration) end diff --git a/spec/presenters/partially_signed_in_modal_presenter_spec.rb b/spec/presenters/partially_signed_in_modal_presenter_spec.rb index 0c2c4479d7e..8949e0fa53f 100644 --- a/spec/presenters/partially_signed_in_modal_presenter_spec.rb +++ b/spec/presenters/partially_signed_in_modal_presenter_spec.rb @@ -4,7 +4,8 @@ include ActionView::Helpers::SanitizeHelper let(:expiration) { Time.zone.now + 1.minute + 1.second } - let(:view_context) { ActionController::Base.new } + let(:lookup_context) { ActionView::LookupContext.new(ActionController::Base.view_paths) } + let(:view_context) { ActionView::Base.new(lookup_context, {}, nil) } subject(:presenter) do PartiallySignedInModalPresenter.new(view_context: view_context, expiration: expiration) end From 94282af2828b360d4aa02ed1e13fb1065dbd5faf Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 14 Mar 2022 11:26:11 -0400 Subject: [PATCH 06/10] Convert dataset to getAttribute Seemingly more compatible with element lifecycle, where dataset can be undefined prior to element being connected. For example, when `define`-ing the custom element, the attributeChangedCallback is triggered, but the element isn't yet mounted. --- .../packages/countdown-element/index.ts | 25 +++---------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/app/javascript/packages/countdown-element/index.ts b/app/javascript/packages/countdown-element/index.ts index 29eea15ff1a..ec720fd2795 100644 --- a/app/javascript/packages/countdown-element/index.ts +++ b/app/javascript/packages/countdown-element/index.ts @@ -1,27 +1,8 @@ import { t } from '@18f/identity-i18n'; -export interface CountdownElementDataset { - /** - * ISO8601-formatted date string for countdown expiration time. - */ - expiration: string; - - /** - * Interval at which text is updated, in milliseconds. - */ - updateInterval: string; - - /** - * Whether to start the countdown as soon as the element is connected. - */ - startImmediately: 'true' | 'false'; -} - export class CountdownElement extends HTMLElement { #pollIntervalId?: number; - dataset: CountdownElementDataset & DOMStringMap; - static get observedAttributes() { return ['data-expiration']; } @@ -43,7 +24,7 @@ export class CountdownElement extends HTMLElement { } get expiration(): Date { - return new Date(this.dataset.expiration); + return new Date(this.getAttribute('data-expiration')!); } set expiration(expiration: Date) { @@ -55,11 +36,11 @@ export class CountdownElement extends HTMLElement { } get updateInterval(): number { - return Number(this.dataset.updateInterval); + return Number(this.getAttribute('data-update-interval')); } get startImmediately(): boolean { - return this.dataset.startImmediately === 'true'; + return this.getAttribute('data-start-immediately') === 'true'; } start(): void { From 2fc904ed9c83fa29bfbc15ff9786c1f769412251 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 14 Mar 2022 11:40:40 -0400 Subject: [PATCH 07/10] Fix lint errors --- app/javascript/packages/countdown-element/index.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/javascript/packages/countdown-element/index.spec.ts b/app/javascript/packages/countdown-element/index.spec.ts index 519a2b65377..939b87f6dfd 100644 --- a/app/javascript/packages/countdown-element/index.spec.ts +++ b/app/javascript/packages/countdown-element/index.spec.ts @@ -1,9 +1,9 @@ import sinon from 'sinon'; import { i18n } from '@18f/identity-i18n'; import { usePropertyValue } from '@18f/identity-test-helpers'; -import { CountdownElement, CountdownElementDataset } from './index'; +import { CountdownElement } from './index'; -const DEFAULT_DATASET: CountdownElementDataset = { +const DEFAULT_DATASET = { updateInterval: '1000', startImmediately: 'true', expiration: new Date().toISOString(), @@ -30,7 +30,7 @@ describe('CountdownElement', () => { clock.restore(); }); - function createElement(dataset: Partial = {}) { + function createElement(dataset = {}) { const element = document.createElement('lg-countdown') as CountdownElement; Object.assign(element.dataset, DEFAULT_DATASET, dataset); document.body.appendChild(element); From 8492b80b28bed3040d502fa80102563d4fe05337 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 14 Mar 2022 12:10:12 -0400 Subject: [PATCH 08/10] Use static variable syntax vs. getter Example snippets often use getter syntax, since it's better supported in older browsers. But since we transpile, and since reasonably new browsers support static variables, we can update to simplify. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/static --- app/javascript/packages/countdown-element/index.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/javascript/packages/countdown-element/index.ts b/app/javascript/packages/countdown-element/index.ts index ec720fd2795..e42fdc7d6e5 100644 --- a/app/javascript/packages/countdown-element/index.ts +++ b/app/javascript/packages/countdown-element/index.ts @@ -3,9 +3,7 @@ import { t } from '@18f/identity-i18n'; export class CountdownElement extends HTMLElement { #pollIntervalId?: number; - static get observedAttributes() { - return ['data-expiration']; - } + static observedAttributes = ['data-expiration']; connectedCallback() { if (this.startImmediately) { From 4258680b736e0fce94f9ff21eced21ae94cca0e5 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 15 Mar 2022 11:06:10 -0400 Subject: [PATCH 09/10] Guarantee idempotence of CountdownElement#start --- .../packages/countdown-element/index.spec.ts | 13 +++++++++++++ app/javascript/packages/countdown-element/index.ts | 1 + 2 files changed, 14 insertions(+) diff --git a/app/javascript/packages/countdown-element/index.spec.ts b/app/javascript/packages/countdown-element/index.spec.ts index 939b87f6dfd..5666ce65051 100644 --- a/app/javascript/packages/countdown-element/index.spec.ts +++ b/app/javascript/packages/countdown-element/index.spec.ts @@ -102,4 +102,17 @@ describe('CountdownElement', () => { expect(element.textContent).to.equal('0 minutes and 2 seconds'); }); + + describe('#start', () => { + it('is idempotent', () => { + const element = createElement({ startImmediately: 'false' }); + + sinon.spy(element, 'setTimeRemaining'); + + element.start(); + element.start(); + + expect(clock.countTimers()).to.equal(1); + }); + }); }); diff --git a/app/javascript/packages/countdown-element/index.ts b/app/javascript/packages/countdown-element/index.ts index e42fdc7d6e5..a107fc05540 100644 --- a/app/javascript/packages/countdown-element/index.ts +++ b/app/javascript/packages/countdown-element/index.ts @@ -42,6 +42,7 @@ export class CountdownElement extends HTMLElement { } start(): void { + this.stop(); this.setTimeRemaining(); this.#pollIntervalId = window.setInterval(() => this.setTimeRemaining(), this.updateInterval); } From 0c4c4e9839ae08ad3b297332a9b20e8156def117 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 15 Mar 2022 13:43:26 -0400 Subject: [PATCH 10/10] Update countdown text via Text#nodeValue **Why**: To restore previous behavior of screen reader atomic announcement including time remaining. --- app/javascript/packages/countdown-element/index.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/javascript/packages/countdown-element/index.ts b/app/javascript/packages/countdown-element/index.ts index a107fc05540..558f162d37e 100644 --- a/app/javascript/packages/countdown-element/index.ts +++ b/app/javascript/packages/countdown-element/index.ts @@ -41,6 +41,14 @@ export class CountdownElement extends HTMLElement { return this.getAttribute('data-start-immediately') === 'true'; } + get #textNode(): Text { + if (!this.firstChild) { + this.appendChild(this.ownerDocument.createTextNode('')); + } + + return this.firstChild as Text; + } + start(): void { this.stop(); this.setTimeRemaining(); @@ -54,7 +62,7 @@ export class CountdownElement extends HTMLElement { setTimeRemaining(): void { const { timeRemaining } = this; - this.textContent = [ + this.#textNode.nodeValue = [ t('datetime.dotiw.minutes', { count: Math.floor(timeRemaining / 60000) }), t('datetime.dotiw.seconds', { count: Math.floor(timeRemaining / 1000) % 60 }), ].join(t('datetime.dotiw.two_words_connector'));