Fix Bugs in the "Copy article URL to clipboard" feature #14242 (#14243)

* change readonly input to p tag

* add pointer cursor and focus on copy post url option

* add Enter keypress eventListener to copy post url option

* add test for enter press event, refractor some test to click on copy post url option instead of just the image

* make copy post url font bold

* add scss class sidebar for dropdown && fix sidebar dropdown reaching under topheader in large screen resolution

* resolve conflict with main

* Update _actions.html.erb

* changin markup to improve accessibility

* make variable name appropriate, switch cypress to imitate user interaction

* remove dump file, fix test to use alias
This commit is contained in:
zeotuan 2021-07-21 00:09:24 +10:00 committed by GitHub
parent e354eb24e2
commit 9dcb332f4a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 52 additions and 45 deletions

View file

@ -17,4 +17,16 @@
max-width: 360px;
width: max-content;
}
&.side-bar {
bottom: 100%;
font-size: var(--fs-base);
padding: var(--su-1);
right: var(--su-1);
@media (min-width: $breakpoint-m) {
right: auto;
bottom: auto;
top: 0;
}
}
}

View file

@ -50,8 +50,8 @@ if (shareDropdownButton.dataset.initialized !== 'true') {
.querySelectorAll('#article-show-more-dropdown [href]')
.forEach((link) => {
link.addEventListener('click', (event) => {
closeDropdown(event)
closeDropdown(event);
// Temporary Ahoy Stats for usage reports
ahoy.track('Post Dropdown', { option: event.target.text.trim() });
});
@ -63,25 +63,19 @@ if (shareDropdownButton.dataset.initialized !== 'true') {
// Initialize the copy to clipboard functionality
function showAnnouncer() {
const { activeElement } = document;
const input =
activeElement.localName === 'clipboard-copy'
? activeElement.querySelector('input')
: document.getElementById('article-copy-link-input');
input.focus();
input.setSelectionRange(0, input.value.length);
document.getElementById('article-copy-link-announcer').hidden = false;
}
function copyArticleLink() {
const inputValue = document.getElementById('article-copy-link-input').value;
Runtime.copyToClipboard(inputValue).then(() => {
const postUrlValue = document
.getElementById('copy-post-url-button')
.getAttribute('data-postUrl');
Runtime.copyToClipboard(postUrlValue).then(() => {
showAnnouncer();
});
}
document
.querySelector('clipboard-copy')
.getElementById('copy-post-url-button')
?.addEventListener('click', copyArticleLink);
// Comment Subscription

View file

@ -32,47 +32,51 @@
<button id="article-show-more-button" aria-controls="article-show-more-dropdown" aria-expanded="false" aria-haspopup="true" class="dropbtn crayons-btn crayons-btn--ghost-dimmed crayons-btn--icon-rounded" aria-label="Share post options">
<%= inline_svg_tag("overflow-horizontal.svg", aria_hidden: true, class: "dropdown-icon crayons-icon", title: "More...") %>
</button>
<div id="article-show-more-dropdown" class="crayons-dropdown p-1 right-1 left-1 s:left-auto bottom-100 m:bottom-auto m:top-0 m:right-auto m:left-100 fs-base">
<clipboard-copy for="article-copy-link-input" aria-live="polite" aria-controls="article-copy-link-announcer" class="dropdown-link-row">
<div class="flex items-center">
<input type="text" id="article-copy-link-input" value="<%= article_url(@article) %>" aria-label="Post URL" class="crayons-textfield" readonly />
<%= inline_svg_tag("copy.svg", aria: true, id: "article-copy-icon", class: "crayons-icon mx-2 shrink-0", title: "Copy article link to the clipboard") %>
</div>
<div id="article-copy-link-announcer" class="crayons-notice crayons-notice--success my-2 p-1" aria-live="polite" hidden>Copied to Clipboard</div>
</clipboard-copy>
<div id="article-show-more-dropdown" class="crayons-dropdown side-bar left-1 s:left-auto m:left-100">
<div>
<button
id="copy-post-url-button"
class="flex justify-between crayons-link crayons-link--block w-100 bg-transparent border-0"
data-postUrl="<%= article_url(@article) %>">
<span class="fw-bold">Copy Post URL</span>
<%= inline_svg_tag("copy.svg", aria_hidden: true, id: "article-copy-icon", class: "crayons-icon mx-2 shrink-0", title: "Copy article link to the clipboard") %>
</button>
<div id="article-copy-link-announcer" aria-live="polite" class="crayons-notice crayons-notice--success my-2 p-1" aria-live="polite" hidden>Copied to Clipboard</div>
</div>
<div class="Desktop-only">
<a
target="_blank"
class="dropdown-link-row crayons-link crayons-link--block"
class="crayons-link crayons-link--block"
rel="noopener"
href='https://twitter.com/intent/tweet?text=<%= u "\"#{@article.title.strip}\" by " %><%= u @article.user.twitter_username ? "@#{@article.user.twitter_username}" : @article.user.name %><%= u " #{Settings::General.twitter_hashtag} " %><%= u " #{article_url(@article)}" %>'>
Share to Twitter
</a>
<a
target="_blank"
class="dropdown-link-row crayons-link crayons-link--block"
class="crayons-link crayons-link--block"
rel="noopener"
href="https://www.linkedin.com/shareArticle?mini=true&url=<%= u article_url(@article) %>&title=<%= u @article.title %>&summary=<%= u @article.description %>&source=<%= u community_name %>">
Share to LinkedIn
</a>
<a
target="_blank"
class="dropdown-link-row crayons-link crayons-link--block"
class="crayons-link crayons-link--block"
rel="noopener"
href="https://www.reddit.com/submit?url=<%= u article_url(@article) %>&title=<%= u @article.title %>">
Share to Reddit
</a>
<a
target="_blank"
class="dropdown-link-row crayons-link crayons-link--block"
class="crayons-link crayons-link--block"
rel="noopener"
href="https://news.ycombinator.com/submitlink?u=<%= u article_url(@article) %>&t=<%= u @article.title %>">
Share to Hacker News
</a>
<a
target="_blank"
class="dropdown-link-row crayons-link crayons-link--block"
class="crayons-link crayons-link--block"
rel="noopener"
href="https://www.facebook.com/sharer.php?u=<%= u article_url(@article) %>">
Share to Facebook
@ -85,7 +89,7 @@
<a href="#" class="dropdown-link-row crayons-link crayons-link--block">Share Post via...</a>
</template>
<a href="/report-abuse" class="dropdown-link-row crayons-link crayons-link--block">Report Abuse</a>
<a href="/report-abuse" class="crayons-link crayons-link--block">Report Abuse</a>
</div>
</div>
</div>

View file

@ -19,19 +19,16 @@ describe('Post sidebar actions', () => {
it('should open and close the share menu for the post', () => {
// Check dropdown is closed by asserting the first option isn't visible
cy.findByRole('img', {
name: /^Copy article link to the clipboard$/i,
}).should('not.exist');
cy.findByRole('button', { name: /^Copy Post URL$/i }).should('not.exist');
cy.findByRole('button', { name: /^Share post options$/i }).as(
'dropdownButton',
);
cy.get('@dropdownButton').click();
cy.findByLabelText('Post URL').should('have.focus');
cy.findByRole('img', {
name: /^Copy article link to the clipboard$/i,
});
cy.findByRole('button', { name: /^Copy Post URL$/i }).as(
'copyPostUrlButton',
);
cy.get('@copyPostUrlButton').should('have.focus');
cy.findByRole('link', { name: /^Share to Twitter$/i });
cy.findByRole('link', { name: /^Share to LinkedIn$/i });
cy.findByRole('link', { name: /^Share to Reddit$/i });
@ -46,9 +43,7 @@ describe('Post sidebar actions', () => {
cy.get('@dropdownButton').click();
// Check dropdown is closed by asserting the first option isn't visible
cy.findByRole('img', {
name: /^Copy article link to the clipboard$/i,
}).should('not.exist');
cy.get('@copyPostUrlButton').should('not.be.visible');
});
it('should close the options dropdown on Escape press, returning focus', () => {
@ -56,10 +51,12 @@ describe('Post sidebar actions', () => {
'dropdownButton',
);
cy.get('@dropdownButton').click();
cy.findByLabelText('Post URL').should('have.focus');
cy.findByRole('button', { name: /^Copy Post URL$/i }).as(
'copyPostUrlButton',
);
cy.get('@copyPostUrlButton').should('have.focus');
cy.get('body').type('{esc}');
cy.findByLabelText('Post URL').should('not.be.visible');
cy.get('@copyPostUrlButton').should('not.be.visible');
cy.get('@dropdownButton').should('have.focus');
});
@ -68,11 +65,11 @@ describe('Post sidebar actions', () => {
'dropdownButton',
);
cy.get('@dropdownButton').click();
cy.findByRole('button', { name: /^Copy Post URL$/i }).as(
'copyPostUrlButton',
);
cy.findByText('Copied to Clipboard').should('not.be.visible');
cy.findByRole('img', {
name: /^Copy article link to the clipboard$/i,
}).click();
cy.get('@copyPostUrlButton').click();
cy.findByText('Copied to Clipboard').should('be.visible');
// Close the dropdown, and reopen it to check the message has disappeared