Improve onboarding “suggested tags” page design (#19475)

* Onboarding tags * Logical design issue fixes * Posts count added * Checkbox design fixes * Optimised css * Nit fixes * Test case added * Reduced data * Nit fix * Added more tests * Nit fix * Tests updatedt
2023-05-19 23:46:24 +05:30 · 2023-05-19 23:46:24 +05:30 · a441358f01
commit a441358f01
parent 375cbfe946
7 changed files with 182 additions and 118 deletions
--- a/app/assets/stylesheets/config/_colors.css
+++ b/app/assets/stylesheets/config/_colors.css
@ -144,6 +144,8 @@
  --tag-bg-hover: rgba(var(--grey-900), 0.05);
  --tag-prefix: rgba(var(--grey-900), 0.6);
  --tag-prefix-hover: rgb(var(--grey-900));
+  --tag-onboarding-bg: var(--white);
+  --tag-onboarding-border: var(--grey-200);

  /* Story */
  --story-comments-bg: rgb(var(--grey-50));
@ -297,6 +299,12 @@

  --list-item-bg-hover: rgb(var(--grey-50));

+  /***********************************************
+  ** Checkbox ************************************
+  ***********************************************/
+
+  --checkbox-default-bg: rgb(var(--grey-50));
+
  /***********************************************
  ** Modals **************************************
  ***********************************************/
--- a/app/assets/stylesheets/preact/onboarding-modal.scss
+++ b/app/assets/stylesheets/preact/onboarding-modal.scss
@ -219,82 +219,91 @@
 .onboarding-tags {
  display: grid;
  grid-template-columns: repeat(2, 1fr);
-  grid-gap: var(--su-4);
+  grid-gap: var(--su-2);
  width: 100%;

  @media screen and (min-width: 800px) {
    grid-template-columns: repeat(3, 1fr);
  }

-  &__button {
-    @extend .crayons-btn;
-    border: 0;
-    display: block;
-    position: absolute;
-    left: var(--su-4);
-    right: var(--su-4);
-    bottom: var(--su-4);
-    width: calc(100% - 32px);
-    transform: translateY(0px);
-    font-family: var(--ff-sans-serif);
-    $button: &;
-
-    @media screen and (min-width: 800px) {
-      transform: translateY(60px);
-    }
-
-    &-default {
-      display: inline-block;
-    }
-
-    &-alt {
-      display: none;
-    }
-  }
-
  &__item {
+    border: 1px solid rgba(var(--tag-onboarding-border), 1);
+    background: var(--tag-onboarding-bg);
    border-radius: var(--radius);
+    padding: var(--su-3) var(--su-4);
    position: relative;
    overflow: hidden;
-    height: 150px;
+    height: 73px;
+    display: flex;
+    flex-direction: row;
+    justify-content: space-between;
+    align-items: center;
    $item: &;

-    &__inner {
-      background: var(--base-10);
-      bottom: 0;
-      font-family: var(--ff-monospace);
-      hyphens: auto;
-      left: 0;
-      padding: var(--su-4);
-      position: absolute;
-      right: 0;
-      top: 0;
-      transition: all ease-in-out 0.1s;
+    input[type='checkbox'] {
+      border-radius: 50%;
+      background-color: var(--checkbox-default-bg);
+      background-image: none;
    }

-    &:hover,
-    &:focus-within {
-      #{$item}__inner {
-        background: transparent;
+    &:hover {
+      background: var(--white);
+      border: 1px solid rgba(var(--accent-brand-rgb), 1);
+      box-shadow: var(--shadow-smooth);
+
+      input[type='checkbox'] {
+        border: 1px solid rgba(var(--accent-brand-rgb), 1);
+      }
+    }
+
+    &__inner {
+      width: 100%;
+      display: flex;
+      flex-direction: row;
+      justify-content: space-between;
+      align-items: center;
+
+      &__content {
+        display: flex;
+        flex-direction: column;
+        justify-content: center;
+        align-items: flex-start;
+        padding: 0px;
+        gap: 4px;
      }

-      .onboarding-tags__button {
-        transform: translateY(0);
+      &__content-name {
+        font-weight: var(--fw-medium);
+        font-size: var(--fs-base);
+        line-height: var(--lh-base);
+        letter-spacing: -0.02em;
+        color: var(--color-primary);
+      }

-        &:hover,
-        &:focus-within {
-          transform: translateY(-2px);
-        }
+      &__content-count {
+        font-weight: var(--fw-normal);
+        font-size: var(--fs-s);
+        line-height: var(--lh-base);
+        letter-spacing: -0.02em;
+        color: var(--tag-prefix);
      }
    }

    &--selected {
-      #{$item}__inner {
-        background: transparent;
+      background: rgba(var(--accent-brand-rgb), 0.1);
+      border: 1px solid rgba(var(--accent-brand-rgb), 1);
+      box-shadow: var(--shadow-smooth);
+
+      &:hover {
+        background: rgba(var(--accent-brand-rgb), 0.1);
+        border: 1px solid rgba(var(--accent-brand-rgb), 1);
+        box-shadow: var(--shadow-smooth);
      }

-      input[type='checkbox']:checked {
-        background: var(--link-branded-color);
+      input[type='checkbox'] {
+        border: 1px solid rgba(var(--accent-brand-rgb), 1);
+        background-color: rgba(var(--accent-brand-rgb), 1);
+        background-image: url("data:image/svg+xml,%3Csvg width='12' height='10' fill='none' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath fill-rule='evenodd' clip-rule='evenodd' d='M11.157.933a.75.75 0 01.077 1.058L4.817 9.407a.75.75 0 01-1.134 0L.766 6.037a.75.75 0 011.135-.982L4.25 7.77l5.85-6.76a.75.75 0 011.057-.077z' fill='%23fff'/%3E%3C/svg%3E");
      }
    }
  }
--- a/app/assets/stylesheets/themes/dark.css
+++ b/app/assets/stylesheets/themes/dark.css
@ -153,6 +153,8 @@
  --tag-bg-hover: rgba(var(--grey-50), 0.05);
  --tag-prefix: rgba(var(--grey-50), 0.6);
  --tag-prefix-hover: rgb(var(--grey-50));
+  --tag-onboarding-bg: var(--grey-900);
+  --tag-onboarding-border: var(--grey-700);

  /* Story */
  --story-comments-bg: rgb(var(--grey-800));
@ -299,6 +301,12 @@
  
  --list-item-bg-hover: rgb(var(--grey-800));
  
+  /***********************************************
+  ** Checkbox ************************************
+  ***********************************************/
+
+  --checkbox-default-bg: rgb(var(--grey-800));
+
  /***********************************************
  ** Modals **************************************
  ***********************************************/
--- a/app/controllers/tags_controller.rb
+++ b/app/controllers/tags_controller.rb
@ -4,6 +4,7 @@ class TagsController < ApplicationController
  after_action :verify_authorized

  ATTRIBUTES_FOR_SERIALIZATION = %i[id name bg_color_hex text_color_hex short_summary badge_id].freeze
+  ONBOARDING_API_ATTRIBUTES = %i[id name taggings_count].freeze
  INDEX_API_ATTRIBUTES = %i[name rules_html short_summary bg_color_hex badge_id].freeze

  TAGS_ALLOWED_PARAMS = %i[
@ -66,8 +67,9 @@ class TagsController < ApplicationController
    skip_authorization

    @tags = Tag.where(name: Settings::General.suggested_tags)
-      .select(ATTRIBUTES_FOR_SERIALIZATION)
+      .select(ONBOARDING_API_ATTRIBUTES)

+    render json: @tags
    set_surrogate_key_header Tag.table_key, @tags.map(&:record_key)
  end

--- a/app/javascript/onboarding/components/FollowTags.jsx
+++ b/app/javascript/onboarding/components/FollowTags.jsx
@ -89,11 +89,7 @@ export class FollowTags extends Component {
      followingStatus = `${selectedTags.length} tags selected`;
    }

-    const classStyle =
-      selectedTags.length > 0
-        ? 'fw-bold color-base-60 fs-base'
-        : 'color-base-60 fs-base';
-    return <p className={classStyle}>{followingStatus}</p>;
+    return <p className="color-base-60 fs-base">{followingStatus}</p>;
  }

  render() {
@ -134,49 +130,39 @@ export class FollowTags extends Component {
                const selected = selectedTags.includes(tag);
                return (
                  <div
+                    data-testid={`onboarding-tag-item-${tag.id}`}
                    className={`onboarding-tags__item ${
-                      selected && 'onboarding-tags__item--selected'
+                      selected ? 'onboarding-tags__item--selected' : ''
                    }`}
-                    style={{
-                      boxShadow: selected
-                        ? `inset 0 0 0 100px ${tag.bg_color_hex}`
-                        : `inset 0 0 0 0px ${tag.bg_color_hex}`,
-                      color: selected ? tag.text_color_hex : '',
-                    }}
+                    aria-label={`Follow ${tag.name}`}
                    key={tag.id}
+                    onClick={() => this.handleClick(tag)}
+                    onKeyDown={(event) => {
+                      // Trigger for enter (13) and space (32) keys
+                      if (event.keyCode === 13 || event.keyCode === 32) {
+                        this.handleClick(tag);
+                      }
+                    }}
+                    tabIndex={0}
+                    role="button"
                  >
                    <div className="onboarding-tags__item__inner">
-                      #{tag.name}
-                      <button
-                        type="button"
-                        onClick={() => this.handleClick(tag)}
-                        className={`onboarding-tags__button  ${
-                          selected &&
-                          'onboarding-tags__button--selected crayons-btn--icon-left'
-                        }`}
-                        aria-pressed={selected}
-                        aria-label={`Follow ${tag.name}`}
-                        style={{
-                          backgroundColor: selected
-                            ? tag.text_color_hex
-                            : tag.bg_color_hex,
-                          color: selected
-                            ? tag.bg_color_hex
-                            : tag.text_color_hex,
-                        }}
-                      >
-                        {selected && (
-                          <svg
-                            width="24"
-                            height="24"
-                            xmlns="http://www.w3.org/2000/svg"
-                            className="crayons-icon"
-                          >
-                            <path d="M9.99999 15.172L19.192 5.979L20.607 7.393L9.99999 18L3.63599 11.636L5.04999 10.222L9.99999 15.172Z" />
-                          </svg>
-                        )}
-                        {selected ? 'Following' : 'Follow'}
-                      </button>
+                      <div className="onboarding-tags__item__inner__content">
+                        <div className="onboarding-tags__item__inner__content-name">
+                          #{tag.name}
+                        </div>
+                        <div className="onboarding-tags__item__inner__content-count">
+                          {tag.taggings_count === 1
+                            ? '1 post'
+                            : `${tag.taggings_count} posts`}
+                        </div>
+                      </div>
+                      <input
+                        class="crayons-checkbox"
+                        type="checkbox"
+                        checked={selected}
+                        disabled
+                      />
                    </div>
                  </div>
                );
--- a/app/javascript/onboarding/components/tests/FollowTags.test.jsx
+++ b/app/javascript/onboarding/components/tests/FollowTags.test.jsx
@ -1,5 +1,5 @@
 import { h } from 'preact';
-import { render } from '@testing-library/preact';
+import { render, waitFor, fireEvent } from '@testing-library/preact';
 import fetch from 'jest-fetch-mock';
 import '@testing-library/jest-dom';

@ -39,18 +39,21 @@ describe('FollowTags', () => {
      id: 715,
      name: 'discuss',
      text_color_hex: '#ffffff',
+      taggings_count: 12,
    },
    {
      bg_color_hex: '#f7df1e',
      id: 6,
      name: 'javascript',
      text_color_hex: '#000000',
+      taggings_count: 0,
    },
    {
      bg_color_hex: '#2a2566',
      id: 630,
      name: 'career',
      text_color_hex: '#ffffff',
+      taggings_count: 1,
    },
  ]);

@ -60,42 +63,43 @@ describe('FollowTags', () => {
    document.body.setAttribute('data-user', getUserData());
  });

-  it('should render the correct tags', async () => {
+  it('should render the correct tags and counts', async () => {
    fetch.mockResponseOnce(fakeTagsResponse);
    const { findByText } = renderFollowTags();
    const javascriptTag = await findByText(/javascript/i);
+    const javascriptCount = await findByText('0 posts');
    const discussTag = await findByText(/discuss/i);
+    const discussCount = await findByText('12 posts');
    const careerTag = await findByText(/career/i);
+    const careerCount = await findByText('1 post');

    expect(javascriptTag).toBeInTheDocument();
+    expect(javascriptCount).toBeInTheDocument();
    expect(discussTag).toBeInTheDocument();
+    expect(discussCount).toBeInTheDocument();
    expect(careerTag).toBeInTheDocument();
+    expect(careerCount).toBeInTheDocument();
  });

  it('should render the correct navigation button on first load', () => {
    fetch.mockResponseOnce(fakeTagsResponse);
-    const { queryByText } = renderFollowTags();
+    const { getByText } = renderFollowTags();

-    expect(queryByText(/skip for now/i)).toBeDefined();
+    expect(getByText(/skip for now/i)).toBeDefined();
  });

-  it('should update the navigation button text, follow status and count when you follow a tag', async () => {
+  it('should update the status and count when you follow a tag', async () => {
    fetch.mockResponse(fakeTagsResponse);

-    const { queryByText, findByText, findAllByText } = renderFollowTags();
-    const followButtons = await findAllByText('Follow');
+    const { getByText, findByTestId } = renderFollowTags();

-    findByText(/skip for now/);
+    const javascriptTag = await findByTestId(`onboarding-tag-item-6`);
+    javascriptTag.click();

-    // click on the first follow button
-    const button = followButtons[0];
-    button.click();
-
-    // it should change to Following and update the count
-    await findByText(/Following/i);
-
-    expect(queryByText(/1 tag selected/i)).toBeDefined();
-    expect(queryByText(/continue/i)).toBeDefined();
+    await waitFor(() =>
+      expect(getByText('1 tag selected')).toBeInTheDocument(),
+    );
+    await waitFor(() => expect(getByText(/continue/i)).toBeInTheDocument());
  });

  it('should render a stepper', () => {
@ -109,4 +113,53 @@ describe('FollowTags', () => {

    expect(queryByTestId('back-button')).toBeDefined();
  });
+
+  it('should call handleClick when enter key is pressed', async () => {
+    fetch.mockResponseOnce(fakeTagsResponse);
+    const { findByTestId, getByText } = renderFollowTags();
+    const javascriptTag = await findByTestId(`onboarding-tag-item-6`);
+
+    // Simulate 'Enter' key press
+    fireEvent.keyDown(javascriptTag, {
+      key: 'Enter',
+      code: 'Enter',
+      keyCode: 13,
+      charCode: 13,
+    });
+
+    await waitFor(() =>
+      expect(getByText('1 tag selected')).toBeInTheDocument(),
+    );
+  });
+
+  it('should call handleClick when space key is pressed', async () => {
+    fetch.mockResponseOnce(fakeTagsResponse);
+    const { findByTestId, getByText } = renderFollowTags();
+    const javascriptTag = await findByTestId(`onboarding-tag-item-6`);
+
+    // Simulate 'Space' key press
+    fireEvent.keyDown(javascriptTag, {
+      key: ' ',
+      code: 'Space',
+      keyCode: 32,
+      charCode: 32,
+    });
+
+    await waitFor(() =>
+      expect(getByText('1 tag selected')).toBeInTheDocument(),
+    );
+  });
+
+  it('should call handleClick and not select the tag when any other key is pressed', async () => {
+    fetch.mockResponseOnce(fakeTagsResponse);
+    const { findByTestId, getByText } = renderFollowTags();
+    const javascriptTag = await findByTestId(`onboarding-tag-item-6`);
+
+    // Simulate 'A' key press
+    fireEvent.keyDown(javascriptTag, { key: 'A', code: 'KeyA', charCode: 65 });
+
+    await waitFor(() =>
+      expect(getByText('0 tags selected')).toBeInTheDocument(),
+    );
+  });
 });
--- a/spec/requests/tags_spec.rb
+++ b/spec/requests/tags_spec.rb
@ -214,12 +214,10 @@ RSpec.describe "Tags", proper_status: true do
      get onboarding_tags_path, headers: headers

      response_tag = response.parsed_body.first
-      expect(response_tag.keys).to match_array(%w[id name bg_color_hex text_color_hex following])
+      expect(response_tag.keys).to match_array(%w[id name taggings_count])
      expect(response_tag["id"]).to eq(tag.id)
      expect(response_tag["name"]).to eq(tag.name)
-      expect(response_tag["bg_color_hex"]).to eq(tag.bg_color_hex)
-      expect(response_tag["text_color_hex"]).to eq(tag.text_color_hex)
-      expect(response_tag[I18n.t("core.following")]).to be_nil
+      expect(response_tag["taggings_count"]).to eq(tag.taggings_count)
    end

    it "returns only suggested tags" do