canonical / ubuntu.com

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

Refresh static contact form for /kernel/real-time #14044

Closed britneywwc closed 2 months ago

britneywwc commented 3 months ago

Done

QA

Issue / Card

Fixes WD-13103

Screenshots

Screenshot 2024-07-09 at 3 33 55 PM Screenshot 2024-07-09 at 3 34 03 PM

Help

QA steps - Commit guidelines

webteam-app commented 3 months ago

Demo

Jenkins

demos.haus

britneywwc commented 2 months ago

@petesfrench @juanruitina This is ready for review

akbarkz commented 2 months ago

@britneywwc SCSS and Prettier CI actions are failing, can you check please?

juanruitina commented 2 months ago

It seems fieldset only picks up legend as the label if it's immediately nested under it, so we will need to be a bit more explicit. aria-labelledby should do the trick (code is just a suggestion, feel free to tweak as you wish)

Current implementation (see accessibility inspector on the right): Screenshot 2024-07-10 at 14 04 32

With aria-labelledby (inspector does pick label): Screenshot 2024-07-10 at 14 06 14

Unrelated: I think you should us p-section--hero in the first section so the spacing is right.

petesfrench commented 2 months ago

@juanruitina This feels weird being able to select both, the versions you use and, that you don't use any. I also wonder if this could lead to funneling problems for the marketing team image

juanruitina commented 2 months ago

I would be OK with disabling "I don't use Ubuntu today" and "I don't know" if any of the fields above them is checked. Similarly, disabling all other fields when either "I don't use Ubuntu today" and "I don't know" are checked.

petesfrench commented 2 months ago

@britneywwc I had a little play around and was able to implement the pre-existing script static-forms.js, this looks for the .js-formfield class name and dynamically adds the heading and user's input to the 'Comments_for_lead' fieldset. I think this is a better approach, as it means we wont have to hard code any forms.

You can see what I did here: https://github.com/canonical/ubuntu.com/pull/14061/files# I have left comments to indicate the changes I made. This is super rough, so if you aren't sure about anything please reach out. Hope this helps.

juanruitina commented 2 months ago

Right now it is possible to select both "I don't use Ubuntu" and "I don't know", I think the other option should also be disabled together with the releases above. Feel free to change to UX+1 once you fix this. Thank you!