Skip to content
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

Thank with Google "Manual" radio button should be a heading + subheading, not a button #5623

Closed
tofumatt opened this issue Jul 28, 2022 · 6 comments
Labels
Module: Thank with Google Thank with Google module related issues P1 Medium priority Type: Bug Something isn't working

Comments

@tofumatt
Copy link
Collaborator

tofumatt commented Jul 28, 2022

Bug Description

In the Figma mocks for Thank with Google, there are two options for placement: "Auto" and "Manual". But really, manual itself isn't an option—it's a heading for three options ("Above the post", "Below the post" and "Below the 1st paragraph").

CleanShot 2022-07-28 at 17 08 19

But "Manual" itself is not actually a choice—it must be one of the three options below it. Having "Manual" be a button is confusing visually and semantically, so instead it should be a heading, eg:

CleanShot 2022-07-28 at 17 09 14

As a workaround we have set the button to disabled, but that isn't great semantically and still causes some visual oddness, especially on hover. It creates an expectation that this can sometimes be clicked and is a confusing UI to present to users. We should keep a clear distinction between interactive and non-interactive elements on the page, so we should change this "Manual" button to a heading/subheading that look the exact same save for missing the radio button.

For context, see this PR comment: #5606 (comment)


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • The "Manual" button in the Thank with Google should be changed to text elements (like a heading and sub-heading or heading and paragraph element) that are styled similarly to the "Auto" radio button option but function as the heading for the three manual button placement options below it.

Implementation Brief

  • In assets/js/modules/thank-with-google/components/common/PositionRadio.js:
    • Swap out the Radio button with the button-placement-manual id with a h5 tag that contains the translated Manual string.
  • In assets/sass/components/thank-with-google/_googlesitekit-twg-position-radio.scss:
    • Inside googlesitekit-twg-position-radio__option class:
      • Add style fot h5 tag to make its style match with the Auto label of the above Radio Button. Some helpful starting point:
        • font-size: 0.875rem;
        • margin: 30px 0 10px 34px;

Test Coverage

  • VRTs may need updating.

QA Brief

Changelog entry

  • Remove radio button from top-level Thank with Google "Manual" position control.
@tofumatt tofumatt added P1 Medium priority Type: Enhancement Improvement of an existing feature Module: Thank with Google Thank with Google module related issues Type: Bug Something isn't working and removed Type: Enhancement Improvement of an existing feature labels Jul 28, 2022
@kuasha420 kuasha420 assigned kuasha420 and unassigned kuasha420 Jul 29, 2022
@techanvil techanvil self-assigned this Aug 2, 2022
@techanvil
Copy link
Collaborator

IB ✅

@techanvil techanvil removed their assignment Aug 2, 2022
@tofumatt tofumatt self-assigned this Aug 2, 2022
@tofumatt tofumatt removed their assignment Aug 2, 2022
@techanvil techanvil assigned techanvil and unassigned techanvil Aug 3, 2022
@mohitwp mohitwp self-assigned this Aug 3, 2022
@mohitwp
Copy link
Collaborator

mohitwp commented Aug 3, 2022

QA Update ✅

Verified;

  • The "Manual" option not have a radio button next to it.
  • The mouse cursor not getting change when hovering over the "Manual" text.

image

@mohitwp mohitwp removed their assignment Aug 3, 2022
@felixarntz
Copy link
Member

@tofumatt Just seeing this now. I tend to agree with your idea, however one could also argue that "Manual" is an option, which triggers three "sub options" to become relevant.

I am not sure about the change here since it looks odd, even if it might also make sense. I'm not sure whether any consultation with UX happened in regards to this problem, but in cases like this, where we clearly diverge from the Figma design, we should generally make sure to do so. What do you think @aaemnnosttv @marrrmarrr?

@aaemnnosttv
Copy link
Collaborator

@felixarntz just seeing this now as well. "Manual" looks odd as a standalone, but it also looks odd as only a group of choices.

I would suggest one of the following:

  • Keep as designed with Manual as a top-level choice, but one that has the sub-choice pre-selected as having blank selections for that group looks strange. These could be hidden or maybe disabled until Manual is selected, at which point if no choice is set it defaults to the first sub-option
  • Flatten the choices into a single level of hierarchy (i.e. all are top-level choices):
    • Auto
      The prompt is automatically placed where it will perform best
    • Manual – Above the post
      The prompt is placed before the main post content
    • Manual – Below the post
      The prompt is placed after the main post content
    • Manual – Below the first paragraph
      The prompt is after the first paragraph in the main post content

Looking at second option above, the Manual prefix is unnecessary and could simply be removed.

@tofumatt
Copy link
Collaborator Author

tofumatt commented Aug 12, 2022

@felixarntz Manual being an option that gets selected with sub-options could work, though with the space and as few options as we have, arguably too many extra clicks when selecting. Or it would auto-select the first one, which might cause users to skip over the "sub-option".

Evan's proposed solution seems the best to me, and while there's a bit of visual/content repetition (the "manual" prefix each time), it's actually more accessible and I'd argue more usable.

I say we go with the flat option proposed by Evan, since it's the easier from both a cognitive, UX, and development perspective. 👍🏻

@tofumatt
Copy link
Collaborator Author

I've filed #5683 as a follow-up issue to discuss the "flat" approach with UX and make the relevant changes. 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: Thank with Google Thank with Google module related issues P1 Medium priority Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants