From 9dcb332f4abe8531ef841442e68f910ffa93735a Mon Sep 17 00:00:00 2001
From: zeotuan <48720253+zeotuan@users.noreply.github.com>
Date: Wed, 21 Jul 2021 00:09:24 +1000
Subject: [PATCH] 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
---
 .../stylesheets/components/dropdowns.scss     | 12 +++++++
 app/javascript/packs/articlePage.jsx          | 20 ++++-------
 app/views/articles/_actions.html.erb          | 32 ++++++++++--------
 .../articleFlows/postSidebarActions.spec.js   | 33 +++++++++----------
 4 files changed, 52 insertions(+), 45 deletions(-)

diff --git a/app/assets/stylesheets/components/dropdowns.scss b/app/assets/stylesheets/components/dropdowns.scss
index 6293a3f0a..fb4c6435a 100644
--- a/app/assets/stylesheets/components/dropdowns.scss
+++ b/app/assets/stylesheets/components/dropdowns.scss
@@ -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;
+    }
+  }
 }
diff --git a/app/javascript/packs/articlePage.jsx b/app/javascript/packs/articlePage.jsx
index 05da2119a..c6566e81a 100644
--- a/app/javascript/packs/articlePage.jsx
+++ b/app/javascript/packs/articlePage.jsx
@@ -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
diff --git a/app/views/articles/_actions.html.erb b/app/views/articles/_actions.html.erb
index 11b4a76f2..7fc383129 100644
--- a/app/views/articles/_actions.html.erb
+++ b/app/views/articles/_actions.html.erb
@@ -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>
diff --git a/cypress/integration/articleFlows/postSidebarActions.spec.js b/cypress/integration/articleFlows/postSidebarActions.spec.js
index 7036fb286..1ee918a18 100644
--- a/cypress/integration/articleFlows/postSidebarActions.spec.js
+++ b/cypress/integration/articleFlows/postSidebarActions.spec.js
@@ -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