Feed Example: Move display of example from separate page into standard APG example page

[ariellalgilmore]

Closes: #2747

This moves the feed example inside the page using an iframe

• add an iframe linking to the frame-display.html
• deleted unused imgs folder
• updated tests

worked on with @andreancardona 🎉 !

Preview link

Infinite Scrolling Feed Example | APG | WAI | W3C

Review checklist

Reviewers: To learn what needs to be covered by each review, Follow the link for the type of review to which you are assigned.

• Editorial review by @mcking65 (plus keyboard navigation)
• Functional review by @jongund
• Visual design review by @a11ydoer
• Accessibility review @a11ydoer
• Code review by @howard-e
• Test review by @howard-e

---

WAI Preview Link (Last built on Tue, 21 May 2024 13:38:10 GMT).

[mcking65]

Hi @ariellalgilmore, it seems atypical that a feed would be in a dialog and more likely that it would be integrated into the body of a page. I am curious what inspired the dialog approach? We can discuss in tomorrow's meeting ... hopefully.

[ariellalgilmore]

Hi @ariellalgilmore, it seems atypical that a feed would be in a dialog and more likely that it would be integrated into the body of a page. I am curious what inspired the dialog approach? We can discuss in tomorrow's meeting ... hopefully.

Hi @mcking65, the dialog approach was inspired because if we integrated the feed example directly in the page I wasn't sure how users would ever be able to reach the rest of the content within the page since the feed example would continue to add more restaurant recommendations as a user scrolls.

[css-meeting-bot]

The ARIA Authoring Practices (APG) Task Force just discussed Move feed example inside the page.

The full IRC log of that discussion

<jugglinmike> Subtopic: Move feed example inside the page
<jugglinmike> github: https://github.com//pull/2775
<jugglinmike> arigilmore: The original "feed" example is presented in a separate page, and it uses an infinite scroll so that you cannot reach the bottom of the page
<jugglinmike> arigilmore: That's why I placed the feed in a modal dialog, so that the dynamically-generated content wouldn't prevent viewers from accessing the content that follows the example
<jugglinmike> Matt_King: It being in a dialog feels somewhat irregular to me
<jugglinmike> Matt_King: Could we put it in a disclosure, instead?
<jugglinmike> Matt_King: We could also make sure the "collapse" button is fixed to the top of the feed so that it's always present even as you scroll down on the feed example
<jugglinmike> Jem: We could have a "load more" button so that the user can control when more content is inserted
<jugglinmike> Matt_King: But if the example behaved that way, it would no longer be a feed
<jugglinmike> jugglinmike: We could just do this with a fixed-height container whose CSS "overflow-y" property is set to "scroll"
<jugglinmike> arigilmore: I'll experiment with the disclosure element and with the fixed-height element
<jugglinmike> arigilmore: The only con is that neither of these are quite representative of the "feed" instances we've been discussing (Facebook, Amazon, etc.)
<Jem> https://www.w3.org/WAI/ARIA/apg/patterns/button/examples/button/#assistivetechnologysupport
<jugglinmike> jugglinmike: We might also use the example as it exists today, but just insert it inline via an iframe (rather than linking to it as a standalone document). In that form, the nested document would more accurately mimic the examples we've been considering (to arigilmore's point)
<Jem> https://deploy-preview-247--aria-practices.netlify.app/aria/apg/about/contributing/
<jugglinmike> Matt_King: Whatever form this takes, it's going to be a great improvement, arigilmore. We appreciate your work on it!

[a11ydoer]

Thanks for PR, @ariellalgilmore and @andreancardona!

To me, the iframe is broken and it has "Page not found" error, in Feed example page. I think it is due to the incorrect path to "feed-display.html".

Can you check the path to the feed-display.html in the sourcesrc="./feed-display.html" correct? I think feed-display.html file path should be under "example" folder, not under second "feed" folder.

[ariellalgilmore]

Thanks @a11ydoer! Weird the deploy preview is acting different then my local and the testing environment it seems. Deploy preview works now, but the tests are breaking 😞

[a11ydoer]

Thanks @a11ydoer! Weird the deploy preview is acting different then my local and the testing environment it seems. Deploy preview works now, but the tests are breaking 😞

@ariellalgilmore Thanks for fixing the path. It looks good. Don't worry about regression test. It is failing because there is no "feed" role information in the testing file.

[css-meeting-bot]

The ARIA Authoring Practices (APG) Task Force just discussed Move feed example inside the page.

The full IRC log of that discussion

<jugglinmike> Subtopic: Move feed example inside the page
<jugglinmike> github: https://github.com//pull/2775
<jugglinmike> andrea_cardona_: We were working on this yesterday
<jugglinmike> Matt_King: This looks good. It makes me wonder what we want to do with the content in the iframe
<Jem> https://deploy-preview-248--aria-practices.netlify.app/aria/apg/patterns/feed/examples/feed/
<Jem> Preview is available from above url
<jugglinmike> Matt_King: there's a part of me that wishes that the control was not inside of the example. I'm also thinking we should get rid of all the text which is on the example page but not part of the page
<jugglinmike> Matt_King: And we should remove the "main" region within the iframe
<jugglinmike> Matt_King: I'm also wondering about the visual presentation of the example, now
<jongund> https://deploy-preview-248--aria-practices.netlify.app/aria/apg/patterns/feed/examples/feed/
<jugglinmike> [Jem shares their screen]
<jugglinmike> Matt_King: The control I was referring to is labeled "Select article loading delay"
<jugglinmike> Matt_King: The carousel page places a control above the example in a dedicated section titled "Example Options"
<jugglinmike> Matt_King: I'm wondering if we want a similar "Example Options" heading above the example in the Feed Display
<jugglinmike> andrea_cardona_: I think we should do that! ariellalgilmore and I can definitely add that feature in
<jugglinmike> Matt_King: The benefit would be that it would give us a space where we can explain the control
<jugglinmike> jugglinmike: It also more strongly delineates the APG example from the UI for modifying the example's behavior. Folks who are new to this pattern might otherwise be confused about which parts of the UI are part of Feed
<dmontalvo> q+
<jugglinmike> jongund: Can we add the words "infinite scroll" to the title? Maybe "Infinite Scroll Feed Example" ?
<jugglinmike> Matt_King: That's a good idea
<jugglinmike> Matt_King: I think the heading "Recommended Restaurants" should be level 3 (it's currently a level 1 heading)
<jugglinmike> Matt_King: I will write a new description of the "select loading delay" control after we've moved it out of the iframe
<jugglinmike> Jem: What is the accessibilty concern which motivates this control?
<jugglinmike> Matt_King: If you set that value to something exaggerated like 3 seconds, if you're scrolling by navigating by article heading, then the AT shouldn't jump out of the feed, and it should convey aria-busy
<jugglinmike> Matt_King: the Feed pattern is intended to be a sort of stopgap for the fact that we don't have the larger "virtualized content" issue resolved in ARIA
<jugglinmike> Matt_King: I think it's going to be a long time before we can take that one. Even years away
<jugglinmike> s/that one/that on/
<jugglinmike> Jem: Microsoft has previously come up with a proposal to handle infinite scroll. I don't know if that proposal is still active

[css-meeting-bot]

The ARIA Authoring Practices (APG) Task Force just discussed PR 2775: Feed example changes.

The full IRC log of that discussion

<jugglinmike> Topic: PR 2775: Feed example changes
<jugglinmike> Matt_King: Because neither Andrea nor Ariella is here, I think we should skip this today
<jugglinmike> github: https://github.com//pull/2775

[css-meeting-bot]

The ARIA Authoring Practices (APG) Task Force just discussed PR 2775: Feed example changes.

The full IRC log of that discussion

<jugglinmike> Topic: PR 2775: Feed example changes
<jugglinmike> github: https://github.com//pull/2775
<jugglinmike> arigilmore: We updated a few examples so it's inside the page, within an iframe
<jugglinmike> arigilmore: We updated the content so there aren't multiple H1 elements on the page
<Matt_King> Example page in preview: https://deploy-preview-248--aria-practices.netlify.app/aria/apg/patterns/feed/examples/feed/
<jugglinmike> arigilmore: We moved the select element which controls the simulated speed of loading. It's no longer displayed alongside the example--it's now outside of the iframe
<Jem> Title is updated to "Infinite Scrolling Feed Example"
<jugglinmike> Matt_King: I see the title tag now says "Infinite Scrolling Feed Example". I remember discussing whether it should be "infinite" or "infinitely". I think the title you have here is fine
<jugglinmike> Matt_King: On the pages where we have example instructions or options, we had a dedicated section for those above the section labeled "example" (eg on "carousel" or "Tree grid")
<jugglinmike> arigilmore: Ah, I see--the sections titled "Example options"
<jugglinmike> arigilmore: Along with a little note about what the option does
<jugglinmike> Jem: When we change the timing option, do we need a reload native link?
<jugglinmike> Matt_King: JAWS handles the change gracefully
<jugglinmike> Jem: Is there any visual indication that more items are loading?
<jugglinmike> arigilmore: No, though you can see the size of the scrollbar change
<jugglinmike> Matt_King: This is awesome work!
<jugglinmike> arigilmore: The only other issue was that the tests weren't working
<jugglinmike> arigilmore: The tests fail because it can't find the page
<jongund> I have to leave a little early today.
<jugglinmike> jugglinmike: this might be related to WebDriver, since you need to explicitly manage context when traversing between documents (e.g. into an iframe)
<jugglinmike> arigilmore: Ah, that sounds familiar. I'll try again and ping you on the issue if I'm still having trouble
<jugglinmike> Matt_King: Once we address these two issues, we can add reviewers in the next meeting

[a11ydoer]

High contrast support with forced-colors is functioning effectively.

[a11ydoer]

This is the approval for the corrected directory path.

[a11ydoer]

@ariellalgilmore @mcking65 I am really sorry for last min notice. The example seem to fail the color contrast ratio in normal text size. The current font color is #777777 and backgroud color is # FAFAFA. The contrast ratio is only 4.29:1, not meeting 4.5:1

if you change the font color as the black in feedDisplay.css, line 67 insteand of #777777, this will have enough high contrast.

.location-block {
  color: #777;

[a11ydoer]

@ariellalgilmore one more thing, target size minimum rule, WCAG SC 2.5.8 AA.
The bookmark button height does not meet target size minimum requirment. Can you increase its height to 24px?

[ariellalgilmore]

@mcking65 looks like other PR's are having the same issue: #2902. I did update my PR to latest and still have the same issue

[css-meeting-bot]

The ARIA Authoring Practices (APG) Task Force just discussed Feed example update.

The full IRC log of that discussion

<jugglinmike> Topic: Feed example update
<jugglinmike> github: https://github.com//pull/2775
<jugglinmike> Matt_King: While testing, I was running into trouble with CTRL+End
<jugglinmike> Matt_King: Right now, since the CTRL+End is specifically targetting the delay slider, it's moving focus to before the feed instead of after the feed
<jugglinmike> arigilmore: Instead of the "terms of use" button? We added that button so there was somewhere else to go
<jugglinmike> Matt_King: Ah, I didn't realize that we had added that. That actually makes the problem simpler to solve than I thought!
<jugglinmike> Matt_King: Putting the focus there would enable that CTRL+End key to work whether its in CodePen or not
<jugglinmike> Matt_King: Oh, we don't have CodePen on this one
<jugglinmike> Matt_King: I don't know if we can. We don't have to make that part of this pull request, though
<jugglinmike> Jem: I can open an issue about adding CodePen to this page.
<jugglinmike> Matt_King: That CTRL+End behavior was the only problem that I had found
<jugglinmike> arigilmore: I'm not sure how much work it will take to fix that. I thought it was working a while ago; I haven't looked in some time, so I wonder if something broke recently
<jugglinmike> arigilmore: There is a regression test. In my "feed" test changes, it checks for the "CTRL+End" keys
<jugglinmike> Matt_King: CTRL+End is clearly taking me to the "delay" selector
<jugglinmike> arigilmore: Okay, I'll take a look
<jugglinmike> Matt_King: We probably need to merge the "main" branch into this branch for these tests to pass
<jugglinmike> arigilmore: The pull request branch is already up-to-date
<jugglinmike> arigilmore: Maybe I have to run the coverage report locally and update it myself
<jugglinmike> howard-e: That seems to be the case, but I'm not understanding why you have to do that...
<jugglinmike> howard-e: The regression in the other failing test is, I think, something I've been seeing recently. I attempted to fix it in a patch I pushed yesterday
<jugglinmike> howard-e: Alex is reviewing that fix, now
<jugglinmike> Matt_King: If arigilmore can figure out what's going on with CTRL+End, then I think this is ready for merge
<jugglinmike> Matt_King: I'm excited to get "feed" moving again. I think there are more steps for feed

[ariellalgilmore]

Hi, I updated the PR today so that it the CTRL-END would go to the "terms-of-use" button, but the deploy preview is throwing an error for me: https://github.com/w3c/wai-aria-practices/actions/runs/8900404954/job/24449218445. Not sure if it has to do with this PR...

cc: @mcking65 @howard-e appreciate your eyes on this 👀

[mcking65]

@ariellalgilmore

I merged #2997 and then merged main into this branch. all tests pass now.

However, ctrl+end is still going to the delay input for me.

[ariellalgilmore]

Thanks @mcking65! As mentioned above: #2775 (comment), the deploy preview is failing because of this error:

Error: Could not determine content type for file at /home/runner/work/wai-aria-practices/wai-aria-practices/_external/aria-practices/content/shared/templates/read-this-first.html
This file may not follow established conventions.
at determineContentType (/home/runner/work/wai-aria-practices/wai-aria-practices/scripts/pre-build/library/determineContentType.js:47:9)
at rewriteSourcePath (/home/runner/work/wai-aria-practices/wai-aria-practices/scripts/pre-build/library/rewritePath.js:24:23)
at recursivelyCopyAllContent (/home/runner/work/wai-aria-practices/wai-aria-practices/scripts/pre-build/library/recursivelyCopyAllContent.js:19:27)
at async preBuild (/home/runner/work/wai-aria-practices/wai-aria-practices/scripts/pre-build/pre-build.js:19:3)

Node.js v18.20.2
Error: Process completed with exit code 1.

[mcking65]

@ariellalgilmore .... Oh, I now see that error. I thought if the preview failed, one of the checks would fail so when I saw the heading with all 25 checks passed, I assumed it was good. I now see that there is a preview build error at the end of the top comment.

@howard-e, please look into this!

[howard-e]

@howard-e, please look into this!

@ariellalgilmore @mcking65 this should now be resolved! I manually updated the build repository's generated PR for this work.

It was due to the generated PR falling out of sync in the build repository. This is already reported in w3c/wai-aria-practices#219 and it can easily happen with older PRs. Maybe some priority needs to be applied to that in the near future.

.... Oh, I now see that error. I thought if the preview failed, one of the checks would fail so when I saw the heading with all 25 checks passed, I assumed it was good. I now see that there is a preview build error at the end of the top comment.

Right, although most would check the build checks first so this does give a false impression that it was successful. I've created #3016 to track this.

[mcking65]

Thank you @ariellalgilmore for all the work on this!