Add separate section for article link to improve accessibility in the mod center (#14312)

* Initial structure for article heading insertion * Fix clashing class names * Draft: Switch to details element + some logic refactor, some specs failing * Fix failing specs * Fix compatibility issues with Safari * Refactor code * Shift app logic to id attribute * Code improvements
2021-08-11 13:00:12 +05:30 · 2021-08-11 13:00:12 +05:30 · 71fd773cf3
commit 71fd773cf3
parent f4a0d60ddf
5 changed files with 131 additions and 83 deletions
--- a/app/assets/stylesheets/views/mod-center.scss
+++ b/app/assets/stylesheets/views/mod-center.scss
@ -43,19 +43,33 @@
  align-items: center;
  background-color: var(--base-inverted);
  color: var(--base-100);
-  display: grid;
-  grid-column-gap: var(--su-2);
-  grid-template-columns: 1fr;
-  grid-template-rows: auto;
-  grid-row-gap: var(--su-1);
-  padding: var(--su-4);
  text-align: left;
  width: 100%;

-  @media (min-width: $breakpoint-m) {
-    grid-template-columns: 4fr 2fr 1fr;
-    grid-row-gap: 0;
-    grid-template-rows: 1fr;
+  > summary {
+    &::-webkit-details-marker {
+      display: none;
+    }
+
+    &::marker {
+      content: '';
+    }
+
+    .article-details-container {
+      cursor: pointer;
+      display: grid;
+      grid-column-gap: var(--su-2);
+      grid-template-columns: 1fr;
+      grid-template-rows: auto;
+      grid-row-gap: var(--su-1);
+      padding: var(--su-4);
+
+      @media (min-width: $breakpoint-m) {
+        grid-template-columns: 4fr 2fr 1fr;
+        grid-row-gap: 0;
+        grid-template-rows: 1fr;
+      }
+    }
  }

  &:hover {
@ -105,6 +119,7 @@
    grid-column-end: 4;
    width: 100%;
    display: flex;
+    flex-direction: column;

    &.opened {
      margin-top: var(--su-4);
@ -116,14 +131,25 @@
      border: none;
    }

-    .article-iframe {
+    .article-referrer-heading {
+      padding: var(--su-4);
      width: 100%;
-      height: 650px; // height is arbitrary, will be removed for modal-esque view
    }

-    .actions-panel-iframe {
-      width: 60%;
-      height: 650px; // height is arbitrary, will be removed for modal-esque view
+    .iframes-container {
+      display: flex;
+      height: 650px;
+      width: 100%;
+
+      .article-iframe {
+        width: 100%;
+        height: 650px; // height is arbitrary, will be removed for modal-esque view
+      }
+
+      .actions-panel-iframe {
+        width: 60%;
+        height: 650px; // height is arbitrary, will be removed for modal-esque view
+      }
    }
  }
 }
--- a/app/javascript/modCenter/tests/moderationArticles.test.jsx
+++ b/app/javascript/modCenter/tests/moderationArticles.test.jsx
@ -1,5 +1,5 @@
 import { h } from 'preact';
-import { render, fireEvent } from '@testing-library/preact';
+import { render, fireEvent, waitFor } from '@testing-library/preact';
 import { ModerationArticles } from '../moderationArticles';

 const getTestArticles = () => {
@ -20,8 +20,7 @@ const getTestArticles = () => {
      id: 2,
      title:
        'An article title that is quite very actually rather extremely long with all things considered',
-      path:
-        'an-article-title-that-is-quite-very-actually-rather-extremely-long-with-all-things-considered-fi8',
+      path: 'an-article-title-that-is-quite-very-actually-rather-extremely-long-with-all-things-considered-fi8',
      published_at: '2019-06-24T09:32:10.590Z',
      cached_tag_list: '',
      user: {
@ -59,29 +58,37 @@ describe('<ModerationArticles />', () => {
    expect(listOfArticles.length).toEqual(2);
  });

-  it('renders the iframes on click', () => {
+  it('renders the iframes on click', async () => {
    const { getByTestId } = render(<ModerationArticles />);
    const singleArticle = getByTestId('mod-article-1');
-    singleArticle.click();
-    const iframes = singleArticle.getElementsByTagName('iframe');
-    expect(iframes.length).toEqual(2);
+    const summarySection = singleArticle.getElementsByTagName('summary')[0];
+    summarySection.click();
+    await waitFor(() => {
+      const iframes = singleArticle.getElementsByTagName('iframe');
+      expect(iframes.length).toEqual(2);
+    });
  });

-  it('toggles the "opened" class when opening or closing an article', () => {
+  it('toggles the "opened" class when opening or closing an article', async () => {
    const { getByTestId } = render(<ModerationArticles />);
    const singleArticle = getByTestId('mod-article-2');
+    const summarySection = singleArticle.getElementsByTagName('summary')[0];

-    fireEvent.click(singleArticle);
-    expect(
-      singleArticle.getElementsByClassName('article-iframes-container')[0]
-        .classList,
-    ).toContain('opened');
+    fireEvent.click(summarySection);
+    await waitFor(() => {
+      expect(
+        singleArticle.getElementsByClassName('article-iframes-container')[0]
+          .classList,
+      ).toContain('opened');
+    });

-    fireEvent.click(singleArticle);
-    expect(
-      singleArticle.getElementsByClassName('article-iframes-container')[0]
-        .classList,
-    ).not.toContain('opened');
+    fireEvent.click(summarySection);
+    await waitFor(() => {
+      expect(
+        singleArticle.getElementsByClassName('article-iframes-container')[0]
+          .classList,
+      ).not.toContain('opened');
+    });
  });

  it('adds the FlagUser Modal HTML associated with author when article opened', async () => {
@ -93,8 +100,9 @@ describe('<ModerationArticles />', () => {
    ).toBeNull();

    const singleArticle = getByTestId(`mod-article-${expectedArticleId}`);
+    const summarySection = singleArticle.getElementsByTagName('summary')[0];

-    singleArticle.click();
+    summarySection.click();

    // We need the iframe to load first before checking for the modal having been loaded.
    await findByTestId(`mod-iframe-${expectedArticleId}`);
--- a/app/javascript/modCenter/moderationArticles.jsx
+++ b/app/javascript/modCenter/moderationArticles.jsx
@ -7,35 +7,47 @@ export class ModerationArticles extends Component {
      document.getElementById('mod-index-list').dataset.articles,
    ),
    prevSelectedArticleId: undefined,
-    selectedArticleId: undefined,
  };

-  toggleArticle = (id, path) => {
+  toggleArticle = (id, title, path) => {
    const { prevSelectedArticleId } = this.state;
    const selectedArticle = document.getElementById(`article-iframe-${id}`);
+    const selectedDetailsPanel = document.getElementById(`mod-article-${id}`);

    if (prevSelectedArticleId > 0) {
-      document.getElementById(
-        `article-iframe-${prevSelectedArticleId}`,
-      ).innerHTML = '';
+      if (selectedDetailsPanel.getAttribute('open') !== null) {
+        if (prevSelectedArticleId !== id) {
+          document
+            .getElementById(`mod-article-${prevSelectedArticleId}`)
+            ?.removeAttribute('open');
+        }
+      } else {
+        document.getElementById(`article-iframe-${id}`).innerHTML = '';
+      }
    }

-    this.setState({ selectedArticleId: id, prevSelectedArticleId: id });
+    if (selectedDetailsPanel.getAttribute('open') !== null) {
+      selectedArticle.innerHTML = `
+      <div class="article-referrer-heading">
+        <a class="article-title-link fw-bold" href=${path}>
+          ${title}
+        </a>
+      </div>
+      <div class="iframes-container">
+        <iframe class="article-iframe" src="${path}"></iframe>
+        <iframe data-testid="mod-iframe-${id}" id="mod-iframe-${id}" class="actions-panel-iframe" id="mod-iframe-${id}" src="${path}/actions_panel"></iframe>
+      </div>`;

-    if (
-      id === prevSelectedArticleId &&
-      document.getElementsByClassName('opened').length > 0
-    ) {
-      selectedArticle.classList.remove('opened');
-      return;
+      this.setState({ prevSelectedArticleId: id });
+    } else {
+      document
+        .getElementById(`article-iframe-${id}`)
+        .classList.remove('opened');
    }
-
-    selectedArticle.classList.add('opened');
-    selectedArticle.innerHTML = `<iframe class="article-iframe" src="${path}"></iframe><iframe data-testid="mod-iframe-${id}" class="actions-panel-iframe" id="mod-iframe-${id}" src="${path}/actions_panel"></iframe>`;
  };

  render() {
-    const { articles, selectedArticleId } = this.state;
+    const { articles, prevSelectedArticleId } = this.state;

    return (
      <div className="moderation-articles-list">
@ -57,7 +69,7 @@ export class ModerationArticles extends Component {
              key={id}
              publishedAt={publishedAt}
              user={user}
-              articleOpened={id === selectedArticleId}
+              articleOpened={id === prevSelectedArticleId}
              toggleArticle={this.toggleArticle}
            />
          );
--- a/app/javascript/modCenter/singleArticle/tests/singleArticle.test.jsx
+++ b/app/javascript/modCenter/singleArticle/tests/singleArticle.test.jsx
@ -1,7 +1,7 @@
 /* eslint-disable jest/expect-expect */
 import { h, Fragment } from 'preact';
 import { axe } from 'jest-axe';
-import { render, getNodeText } from '@testing-library/preact';
+import { render, getNodeText, waitFor } from '@testing-library/preact';
 import { SingleArticle } from '../index';

 const getTestArticle = () => ({
@ -63,7 +63,7 @@ describe('<SingleArticle />', () => {
      </Fragment>,
    );
    const text = getNodeText(
-      container.getElementsByClassName('article-title-link')[0],
+      container.getElementsByClassName('article-title-heading')[0],
    );
    expect(text).toContain(getTestArticle().title);
  });
@ -197,9 +197,10 @@ describe('<SingleArticle />', () => {
      </Fragment>,
    );

-    const button = getByTestId(`mod-article-${article.id}`);
-    button.click();
+    const detailsElement = getByTestId(`mod-article-${article.id}`);
+    const summarySection = detailsElement.getElementsByTagName('summary')[0];
+    summarySection.click();

-    expect(toggleArticle).toHaveBeenCalledTimes(1);
+    waitFor(() => expect(toggleArticle).toHaveBeenCalledTimes(1));
  });
 });
--- a/app/javascript/modCenter/singleArticle/index.jsx
+++ b/app/javascript/modCenter/singleArticle/index.jsx
@ -5,11 +5,10 @@ import { FlagUserModal } from '../../packs/flagUserModal';
 import { formatDate } from './util';

 export class SingleArticle extends Component {
-  activateToggle = (e) => {
-    e.preventDefault();
-    const { id, path, toggleArticle } = this.props;
+  activateToggle = () => {
+    const { id, title, path, toggleArticle } = this.props;

-    toggleArticle(id, path);
+    toggleArticle(id, title, path);
  };

  tagsFormat = (tag, key) => {
@ -50,36 +49,38 @@ export class SingleArticle extends Component {
            <FlagUserModal moderationUrl={path} authorId={user.id} />,
            document.getElementsByClassName('flag-user-modal-container')[0],
          )}
-        <button
+        <details
+          id={`mod-article-${id}`}
          data-testid={`mod-article-${id}`}
-          type="button"
          className="moderation-single-article"
-          onClick={this.activateToggle}
+          onToggle={this.activateToggle}
        >
-          <span className="article-title">
-            <header>
-              <h3 className="fs-base fw-bold lh-tight">
-                <a className="article-title-link" href={path}>
-                  {title}
-                </a>
-              </h3>
-            </header>
-            {tags}
-          </span>
-          <span className="article-author">
-            {newAuthorNotification}
-            {user.name}
-          </span>
-          <span className="article-published-at">
-            <time dateTime={publishedAt}>{formatDate(publishedAt)}</time>
-          </span>
+          <summary>
+            <div className="article-details-container">
+              <span className="article-title">
+                <header>
+                  <h3 className="fs-base fw-bold lh-tight article-title-heading">
+                    {title}
+                  </h3>
+                </header>
+                {tags}
+              </span>
+              <span className="article-author">
+                {newAuthorNotification}
+                {user.name}
+              </span>
+              <span className="article-published-at">
+                <time dateTime={publishedAt}>{formatDate(publishedAt)}</time>
+              </span>
+            </div>
+          </summary>
          <div
-            className={`article-iframes-container ${
-              articleOpened ? 'opened' : ''
+            className={`article-iframes-container${
+              articleOpened ? ' opened' : ''
            }`}
            id={`article-iframe-${id}`}
          />
-        </button>
+        </details>
      </Fragment>
    );
  }