-
Notifications
You must be signed in to change notification settings - Fork 166
LG-4682: Resolve confirmation step button scroll-to-top, role #5705
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8d29077
7e4c7d9
bd80d83
23288f5
9813ae4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| <%= button_tag(content, **tag_options, type: tag_type, class: css_class) %> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| class ButtonComponent < BaseComponent | ||
| attr_reader :type, :outline, :tag_options | ||
|
|
||
| DEFAULT_BUTTON_TYPE = :button | ||
|
|
||
| def initialize(outline: false, **tag_options) | ||
| @outline = outline | ||
| @tag_options = tag_options | ||
| end | ||
|
|
||
| def css_class | ||
| classes = ['usa-button', *tag_options[:class]] | ||
| classes << 'usa-button--outline' if outline | ||
| classes | ||
| end | ||
|
|
||
| def tag_type | ||
| tag_options.fetch(:type, DEFAULT_BUTTON_TYPE) | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| import { loadPolyfills } from '@18f/identity-polyfill'; | ||
|
|
||
| loadPolyfills(['custom-elements', 'clipboard']) | ||
| .then(() => import('@18f/identity-clipboard-button')) | ||
| .then(({ ClipboardButton }) => customElements.define('lg-clipboard-button', ClipboardButton)); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| class ClipboardButtonComponent < ButtonComponent | ||
| attr_reader :clipboard_text, :tag_options | ||
|
|
||
| def initialize(clipboard_text:, **tag_options) | ||
| super(**tag_options) | ||
|
|
||
| @clipboard_text = clipboard_text | ||
| @tag_options = tag_options | ||
| end | ||
|
|
||
| def call | ||
| content_tag(:'lg-clipboard-button', super, data: { clipboard_text: clipboard_text }) | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| export class ClipboardButton extends HTMLElement { | ||
| connectedCallback() { | ||
| /** @type {HTMLButtonElement?} */ | ||
| this.button = this.querySelector('button'); | ||
|
|
||
| this.button?.addEventListener('click', () => this.writeToClipboard()); | ||
| } | ||
|
|
||
| /** | ||
| * Returns the text to be copied to the clipboard. | ||
| * | ||
| * @return {string} | ||
| */ | ||
| get clipboardText() { | ||
| return this.dataset.clipboardText || ''; | ||
| } | ||
|
|
||
| /** | ||
| * Writes the element's clipboard text to the clipboard. | ||
| */ | ||
| writeToClipboard() { | ||
| navigator.clipboard.writeText(this.clipboardText); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| import sinon from 'sinon'; | ||
| import { getByRole } from '@testing-library/dom'; | ||
| import userEvent from '@testing-library/user-event'; | ||
| import { loadPolyfills } from '@18f/identity-polyfill'; | ||
| import { ClipboardButton } from './index.js'; | ||
|
|
||
| describe('ClipboardButton', () => { | ||
| before(async () => { | ||
| // Necessary until: https://github.com/jsdom/jsdom/issues/1568 | ||
| await loadPolyfills(['clipboard']); | ||
|
|
||
| if (!customElements.get('lg-clipboard-button')) { | ||
| customElements.define('lg-clipboard-button', ClipboardButton); | ||
| } | ||
| }); | ||
|
|
||
| beforeEach(() => { | ||
| sinon.spy(navigator.clipboard, 'writeText'); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| navigator.clipboard.writeText.restore(); | ||
| }); | ||
|
|
||
| function createAndConnectElement({ clipboardText = '' } = {}) { | ||
| const element = document.createElement('lg-clipboard-button'); | ||
| element.setAttribute('data-clipboard-text', clipboardText); | ||
| element.innerHTML = '<button type="button" class="usa-button">Copy</button>'; | ||
| document.body.appendChild(element); | ||
| return element; | ||
| } | ||
|
|
||
| it('copies text to clipboard when clicking its button', () => { | ||
| const clipboardText = 'example'; | ||
| const element = createAndConnectElement({ clipboardText }); | ||
| const button = getByRole(element, 'button'); | ||
|
|
||
| userEvent.click(button); | ||
|
|
||
| expect(navigator.clipboard.writeText).to.have.been.calledWith(clipboardText); | ||
| }); | ||
|
|
||
| it('copies the latest clipboard attribute value after initialization', () => { | ||
| const clipboardText = 'example'; | ||
| const element = createAndConnectElement({ clipboardText }); | ||
| const changedClipbordText = 'example2'; | ||
| element.setAttribute('data-clipboard-text', changedClipbordText); | ||
|
|
||
| const button = getByRole(element, 'button'); | ||
|
|
||
| userEvent.click(button); | ||
|
|
||
| expect(navigator.clipboard.writeText).to.have.been.calledWith(changedClipbordText); | ||
| }); | ||
|
|
||
| context('with nothing to copy', () => { | ||
| it('does writes an empty string to the clipboard', () => { | ||
| const element = createAndConnectElement(); | ||
| const button = getByRole(element, 'button'); | ||
|
|
||
| userEvent.click(button); | ||
|
|
||
| expect(navigator.clipboard.writeText).to.have.been.calledWith(''); | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| { | ||
| "name": "@18f/identity-clipboard-button", | ||
| "version": "1.0.0", | ||
| "private": true | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,22 +8,28 @@ | |
| <div class="full-width-box"> | ||
| <%= render 'partials/personal_key/key', code: code %> | ||
| </div> | ||
| <br> | ||
| <div class="sm-col sm-col-4 center"> | ||
| <%= link_to t('forms.backup_code.download'), idv_download_personal_key_path, | ||
| class: 'usa-button usa-button--outline' %> | ||
| </div> | ||
| <div class="sm-col sm-col-4 center"> | ||
| <%= link_to t('users.personal_key.print'), '#', data: {print: true}, | ||
| class: 'usa-button usa-button--outline' %> | ||
| </div> | ||
| <div class="sm-col sm-col-4 center"> | ||
| <%= link_to t('links.copy'), '#', data: {"clipboard-text": code}, | ||
| class: 'usa-button usa-button--outline clipboard' %> | ||
| <div class="grid-row text-center margin-y-3"> | ||
| <div class="grid-col-12 tablet:grid-col-4 margin-y-1"> | ||
| <%= link_to( | ||
| t('forms.backup_code.download'), | ||
| idv_download_personal_key_path, | ||
| class: 'usa-button usa-button--outline', | ||
| ) %> | ||
| </div> | ||
| <div class="grid-col-12 tablet:grid-col-4 margin-y-1"> | ||
| <%= button_tag( | ||
| t('users.personal_key.print'), | ||
| type: :button, | ||
| data: { print: true }, | ||
| class: 'usa-button usa-button--outline', | ||
| ) %> | ||
| </div> | ||
| <div class="grid-col-12 tablet:grid-col-4 margin-y-1"> | ||
| <%= render ClipboardButtonComponent.new(clipboard_text: code, outline: true) do %> | ||
| <%= t('links.copy') %> | ||
| <% end %> | ||
| </div> | ||
| </div> | ||
| <br> | ||
| <br> | ||
| <br> | ||
|
Comment on lines
-24
to
-26
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😍 |
||
| <div class="sm-col sm-col-2 padding-right-2"> | ||
| <%= image_tag(asset_url('alert/icon-lock-alert-important.svg'), alt: '', size: '80') %> | ||
| </div> | ||
|
|
@@ -43,4 +49,4 @@ | |
| 'data-toggle': 'modal' %> | ||
| </p> | ||
| <%= render 'shared/personal_key_confirmation_modal', code: code, update_path: update_path %> | ||
| <%== javascript_packs_tag_once 'personal-key-page-controller', 'clipboard' %> | ||
| <%== javascript_packs_tag_once 'personal-key-page-controller' %> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| require 'rails_helper' | ||
|
|
||
| RSpec.describe ButtonComponent, type: :component do | ||
| let(:type) { nil } | ||
| let(:outline) { false } | ||
| let(:content) { 'Button' } | ||
| let(:options) do | ||
| { | ||
| outline: outline, | ||
| type: type, | ||
| }.compact | ||
| end | ||
|
|
||
| subject(:rendered) { render_inline ButtonComponent.new(options) { content } } | ||
|
|
||
| it 'renders button content' do | ||
| expect(rendered).to have_content(content) | ||
| end | ||
|
|
||
| it 'renders as type=button' do | ||
| expect(rendered).to have_css('button[type=button]') | ||
| end | ||
|
|
||
| it 'renders with design system classes' do | ||
| expect(rendered).to have_css('button.usa-button') | ||
| end | ||
|
|
||
| context 'with outline' do | ||
| let(:outline) { true } | ||
|
|
||
| it 'renders with design system classes' do | ||
| expect(rendered).to have_css('button.usa-button.usa-button--outline') | ||
| end | ||
| end | ||
|
|
||
| context 'with type' do | ||
| let(:type) { :submit } | ||
|
|
||
| it 'renders as type' do | ||
| expect(rendered).to have_css('button[type=submit]') | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| require 'rails_helper' | ||
|
|
||
| RSpec.describe ClipboardButtonComponent, type: :component do | ||
| let(:clipboard_text) { 'Copy Text' } | ||
| let(:content) { 'Button' } | ||
| let(:tag_options) { {} } | ||
|
|
||
| subject(:rendered) do | ||
| render_inline ClipboardButtonComponent.new(clipboard_text: clipboard_text, **tag_options) do | ||
| content | ||
| end | ||
| end | ||
|
|
||
| it 'renders button content' do | ||
| expect(rendered).to have_content(content) | ||
| end | ||
|
|
||
| it 'renders with clipboard text as data-attribute' do | ||
| expect(rendered).to have_css("lg-clipboard-button[data-clipboard-text='#{clipboard_text}']") | ||
| end | ||
|
|
||
| context 'with tag options' do | ||
| let(:tag_options) { { outline: true } } | ||
|
|
||
| it 'renders button given the tag options' do | ||
| expect(rendered).to have_css('button.usa-button.usa-button--outline') | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,4 +12,26 @@ | |
| end | ||
| end | ||
| end | ||
|
|
||
| context 'with javascript enabled', js: true do | ||
| before do | ||
| page.driver.browser.execute_cdp( | ||
| 'Browser.grantPermissions', | ||
| origin: page.server_url, | ||
| permissions: ['clipboardReadWrite', 'clipboardSanitizedWrite'], | ||
| ) | ||
| end | ||
|
|
||
| after do | ||
| page.driver.browser.execute_cdp('Browser.resetPermissions') | ||
| end | ||
|
|
||
| it 'allows a user to copy the code into the confirmation modal' do | ||
| click_on t('links.copy') | ||
| copied_text = page.evaluate_async_script('navigator.clipboard.readText().then(arguments[0])') | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what's
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
From what I understand, So it ends up being something like... (function(/* callback */) {
navigator.clipboard.readText().then(arguments[0])
})(callback);
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. neat, I wish *they'd call it it something clearer like *they = the webdriver/capybara devs
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Initially I'd hoped it would infer the promise value and treat it as something more like... copied_text = page.evaluate_async_script('navigator.clipboard.readText()')(() => navigator.clipboard.readText())().then(callback); |
||
|
|
||
| code = page.all('[data-personal-key]').map(&:text).join('-') | ||
| expect(copied_text).to eq(code) | ||
| end | ||
| end | ||
| end | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One possible future awkwardness we may run into is that because we're just wrapping the base markup, things like
tag_options[:class]will apply to the child element, not the root. A small difference, but personally I'd expecttag_optionsto apply to the root element.