search

canonical / ubuntu.com

The official website for the Ubuntu operating system
https://ubuntu.com
Other
187 stars 190 forks source link

WD-10729 - update contact us forms #13918

Closed lizzochek closed 1 week ago

lizzochek commented 1 month ago

Done

Updated multiple contact forms. Please see more details in the Jira ticket.

QA

Issue / Card

WD-10729

webteam-app commented 1 month ago

Demo

Jenkins

demos.haus

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 74.12%. Comparing base (0277528) to head (bc3d3e7). Report is 7 commits behind head on main.

:exclamation: Current head bc3d3e7 differs from pull request most recent head a0a9e29

Please upload reports for the commit a0a9e29 to get more accurate results.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #13918 +/- ## ======================================= Coverage 74.12% 74.12% ======================================= Files 107 107 Lines 2864 2864 Branches 957 957 ======================================= Hits 2123 2123 Misses 715 715 Partials 26 26 ```
juanruitina commented 1 month ago

A bigger issue. I don't think we should use the 50/50 split when a modal form has several sections (e.g.). Unlike brochure pages where users might skim, users will likely fill forms sequentially. I'd favour keeping the current simple layout. Currently they are paginated: I don't oppose this, maybe one page per section would be excessive, but at least "About you" section (see it at the end of the copy docs). @lyubomir-popov, what do you think? I'll reach out to Jared too to check with him.

These are the modals:

Also checking with Jared:

lyubomir-popov commented 1 month ago

@juanruitina I think the problem is that it isn't implemented correctly yet, lines missing, double indents, so it looks confusing.

@lizzochek Can we plase have the double grid indent removed, so all text lines up on the left and right, and then ca nwe please have the horizontal rules above each h2? I think then we'd be in a better place to evaluate whether the design is working or not.

lizzochek commented 1 month ago

@lyubomir-popov @juanruitina you can see an updated version in this PR: https://github.com/canonical/canonical.com/pull/1270 I corrected according to @lyubomir-popov comments in there. It is easier to do it in that PR as there is only one form. I will implement it here when finally agreed

lyubomir-popov commented 1 month ago

also, can we please do h4s and change the title of the popup to an h3 - lots of long questions, so we have to adapt image

juanruitina commented 1 month ago

@lyubomir-popov Do we have any research on how 50/50 performs on forms? As mentioned above, I'd expect users to fill a form sequentially, and with the 50/50 split they would end up zigzagging. I think it's OK to have two columns if left is an intro to the form and right the fields, but I'm not convinced about splitting fields like that.

lizzochek commented 1 month ago

@lyubomir-popov updated in https://github.com/canonical/canonical.com/pull/1270

juanruitina commented 1 month ago

Got confirmation from Jared on the two items I mentioned at the end:

carkod commented 3 weeks ago

Can you resolve the conflict, so I can have a clear view of the thank-you page code?

carkod commented 3 weeks ago

There's a bug in https://ubuntu-com-13918.demos.haus/security/esm#get-in-touch (and I guess every page that uses that component):

When I click on one of the options (required) of "What kind of device are you using?*", it doesn't recognize it as selected.

Screenshot 2024-06-17 at 14 35 08

Also another issue is that there are fields that are required, but then there is no asterisk (I couldn't find the corresponding copy doc), such as:

Screenshot 2024-06-17 at 14 42 44

Can you add the is-required classname for those that have the required attribute?

Thanks

juanruitina commented 3 weeks ago

Some more feedback. There are quite a few issues with these forms accessibility wise. This kind of markup is very problematic:

      <div class="row--50-50 p-section--shallow u-no-padding--left u-no-padding--right">
        <div class="col">
          <legend class="u-off-screen">Tell us about your project and team</legend>
          <h4>Tell us about your project</h4>
        </div>
        <div class="col">
          <textarea id="about-your-project" rows="3"></textarea>
        </div>
      </div>

Every input should have a label associated. The H4 should be a label element with H4 styling and refer to the input field.

Also, we should not use that off-screen legend attribute as we do. Legend elements are used to label groups of fields in fieldsets, but we are abusing them for no good reason. Screen readers would read both legend and the h4s, making them redundant. Legend would only make sense here as far as I can see for "About us", that could be the legend for a fieldset with all the fields that we have on the right column. Input labels should be such, not legends. We could use legends to label fieldsets of checkboxes or radio buttons, or an "About us" fieldset with all the input fields that we keep on the right hand side.

Something cleaner would look like this:

      <div class="row--50-50 p-section--shallow u-no-padding--left u-no-padding--right">
        <div class="col">
          <label class="p-heading--4" for="about-your-project">Tell us about your project</label>
        </div>
        <div class="col">
          <textarea name="about-your-project" id="about-your-project" rows="3"></textarea>
        </div>
      </div>

(Still inquiring about the two-column layout, will get back to you today)

lizzochek commented 3 weeks ago

@carkod @juanruitina updated according to your comments

carkod commented 3 weeks ago
  • real-time

The main two bugs are still there? Please check https://github.com/canonical/ubuntu.com/pull/13918#issuecomment-2173436694

carkod commented 3 weeks ago

I've given code +1 but please fix the linting errors.

juanruitina commented 3 weeks ago

These don't seem addressed in demos, can you check?

Following up on the "legend" fields, I see them being useful for groups of checkboxes and radio buttons (should be fieldsets). Both fieldsets and legends have some styles coming from either Vanilla or browser styles. A cleaner markup for checkbox/radio fieldsets would look like this:

      <fieldset class="row--50-50 p-section--shallow u-no-padding" id="kind-of-device">
        <div class="col">
          <legend class="p-heading--4">What kind of device are you using?*</legend>
        </div>
        <div class="col">
          <div class="row u-no-padding">
            <div class="col-4 u-sv3">
              <label class="p-checkbox">
                <input required="" class="p-checkbox__input" type="checkbox" aria-labelledby="desktop/workstation" value="desktop/workstation" onclick="validateCheckbox(event, 'kind-of-device')">
                <span class="p-checkbox__label" id="desktop-workstation">Desktop/workstation</span>
              </label>
              [...]
            </div>
          </div>
        </div>
      </fieldset>

Border would need to be removed from the fieldset, I couldn't find a utility for that. Happy to pair to make this work, some further testing would be useful.

eabashidze commented 3 weeks ago

Hi team, I tested the forms, and how the values sync to Marketo from them. You can see the outcomes in this document (let me know if you need further access). I also ran into one error, which I could not replicate when I re-submitted the form, but I will still upload the photo here: 2024-06-18 17-26-27

laszlokajtar commented 3 weeks ago

@carkod there are multiple Marketo submission and sync issues. my hunch is that it has to do with how the form submission has changed with the addition of the

onsubmit="getCustomFields(event)" attribute

carkod commented 3 weeks ago

getCustomFields(e

What kind of problems are you having? Can you be more specific?

laszlokajtar commented 3 weeks ago

@carkod the document linked by Erekle above has the information. mainly: we are receiving only email addresses in Marketo and not the contents of the rest of the fields even though they appear in the payload. in the case of comments from lead field, the value of that doesn't appear in the payload. i suppose that Marketo submission doesn't wait for the returining of the getCustomFields function.

eabashidze commented 3 weeks ago

Hi team, I have updated the report with today's test results (included UTMs in the test). No forms have synced any fields this time unfortunately, only email addresses (see the sheet - marketo database Jun 20 test).

carkod commented 3 weeks ago

Hi team, I have updated the report with today's test results (included UTMs in the test). No forms have synced any fields this time unfortunately, only email addresses (see the sheet - marketo database Jun 20 test).

@eabashidze Thanks. Are we having the same issues on the live site?

lyubomir-popov commented 1 week ago

Please change the background of all forms from white to #f3f3f3 (we have a class is-paper I believe, pls don't hardcode the hex value)

lyubomir-popov commented 1 week ago

This is not using the 50/50 split - either all should or not IMO.

lyubomir-popov commented 1 week ago

Most of the forms use the outgoing style as opposed to the rebranding, not sure if I'm missing anything?

britneywwc commented 1 week ago

Moving this task to a feature branch, will address the design and form submission issues there.