Video section

[ludoboludo]

Why are these changes introduced?

Fixes #49

Figma reference: video section

What approach did you take?

Reused a most of the code from the Collage section. And added some new styling specific to this section.

Other considerations

What should be the spacing like around the heading when the section is full width ? (Test wider screens too).

Demo links

• Editor

Checklist

• Followed theme code principles
• Linted with Theme Check
• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility

[melissaperreault]

Wondering if this should become an "info" below the input so merchants can read the full sentence?

[ludoboludo]

Good point, so it doesn't go away. And have a default be Video to describe your products or your brand kind of thing ?

[melissaperreault]

I wonder if there should be a default, if so, it would have to be short otherwise merchants can't read the full sentence. Perhaps: Describe your video ?

When we edit alt text of an image this is the info helper text:

Write a brief description of this image to improve search engine optimization (SEO) and accessibility for visually impaired customers. This change will be saved immediately.

@gretchen-willenberg could we improve the Video alt text info text to align with what we have for image alt text (and ignore the SEO part)?

Draft: Describe the video for visually impaired customers using screen readers.

[ludoboludo]

I think the reason why users might choose to use screen readers can be left out. Because it's not necessary always true.

[gretchen-willenberg]

Sure!

1. Updated the Video alt text placeholder copy from repeating Video alt text to Describe the video
2. Agree with your revision to Describe the video for customers using screen readers. so it's Describe the video for visually impaired customers using screen readers.

I think I had some fuzzy old memory about not specifying why some one might choose to use a screen reader, but all research I just skimmed includes at least "visually impaired" if not "blind or visually impaired"--so corrected!

[melissaperreault]

I think the reason why users might choose to use screen readers can be left out. Because it's not necessary always true.

@ludoboludo can you elaborate on this please?

[gretchen-willenberg]

Added some research to the Slack thread and an updated version of copy:
Describe the video to make it accessible for customers using screen readers.

[tauthomas01]

It works great! I left small nitpick comments regarding the code, otherwise the functionality works well.

[melissaperreault]

Thank you Ludo! Apart from the last piece of content being discussed, this looks good to me! 🎉

cc. @nicklepine We didn't include a height setting but would like to observe the usage and feedback before doing so unless we feel strongly about it. Merchants cover image would need to be 16:9. Perhaps we could add the info under the setting... I'll leave it to you!

[PaulNewton]

sections/video.liquid
As I understand #49 this was made to explicitly have "Video" show in the theme designer to lessen merchant confusion looking to add video to complement the the more complex collage section?

By extension does does this mean were also going to have a separate 3D section, PDF section, file section, etc.
Or in future pulls adding the support for that other media then change this to sections/media.liquid(or video-and-media* or video-images-and-media) , with it's schema name to Video and Media to show in the designer (still observing #49).
Or break this back down to block usage?
*Using "media" also reflects how apis and liquid organizes content like video under the media concept, but we would probably still want to have video in the filename for merchant discoverability.

As a developer while sections/video.liquid at least pares down the code to manage for a specific type of content I wouldn't want to support more than a handful of distinct media section files (3d,image,animation,videos) simply because the theme architecture doesn't have a method of reusing the existing code in other sections|blocks.

As theme setup support the 'add section' picker does not categorize sections together it only displays them alphabetically making it easy to miss related sections or increasing the time it takes to explore for the right section capabilities as the list grows so the display name of sections should take this into account.

.

[svinkle]

In the editor, the "Describe the video for customers using screen readers" content is set as the value for the input. This causes some issues:

1. People may think the input is pre-filled and not need to do anything more
2. Setting this as the value forces more work on part of the user to remove the default text
3. Submitting as-is sets the video description as "Describe the video for customers using screen readers" which wouldn't be accurate

This content would ideally be hint text appearing below the input, connected via aria-describedby. (Also, the aria-labelledby is redundant.)

<input
  id="TextSetting-Label2"
  placeholder=""
  autocomplete="off"
  class="Polaris-TextField__Input_30ock"
  type="text"
- aria-labelledby="TextSetting-Label2Label"
+ aria-describedby="TextSetting-Label2Hint"
  aria-invalid="false"
- value="Describe the video for customers using screen readers"
/>
+ <small id="TextSetting-Label2Hint">Describe the video for customers using screen readers</small>

[svinkle]

To make this easier on merchants, would it be possible to:

1. Detect a video URL from YouTube or Vimeo
2. Grab the video id
3. Ping the respective API to grab the video title, and
4. Fill in the input

This would suffice as the video description rather than rely on the merchant to come up with something.

[melissaperreault]

🎉 Looks good, thank you Sofia!