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.

Sign up for GitHub

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

Jump to bottom

Twitter timeline - Content - Add working example with multiple timelines #2

Closed
wants to merge 3 commits into from
Closed

Twitter timeline - Content - Add working example with multiple timelines #2

wants to merge 3 commits into from

Conversation

duboisp
Copy link

@duboisp duboisp commented Dec 6, 2023

No description provided.

EricDunsworth and others added 3 commits November 20, 2023 12:03
@EricDunsworth
Twitter embedded timeline: Add data-* presets
d79ba06 
Specifically:
* data-height="500":
  * WET sets it when:
    * Value is explicitly set to "fb-page"
    * data-tweet-limit is used without data-height
  * Needed to prevent older timeline widget implementations from growing extremely tall and to set a reasonable "generic" height going forward
  * Starting July 21, 2023... timeline widgets stopped honouring tweet limits and began showing 100 tweets at a time ("verified" accounts only)
  * 500 pixels matches the default height of the Facebook page plugin (aka Facebook embedded pages)
* data-lang="zh-cn":
  * WET sets it when the in-page language is "zh-Hans"
  * Twitter only supports "zh-cn" for Chinese (Simplified)
* data-dnt="true":
  * Not used if data-lang="false" is already hardcoded

Related changes:
* Plugin now checks every Twitter link in every plugin container (not just the first link)
* Adjust the loading icon's logic accordingly:
  * Move it into the data-* logic's foreach loop
  * Make its initial if condition check for eventTarget.firstElementChild (i.e. only add a loading icon for the first link in the container)
* Documentation:
  * Add specific links to Twitter timelines/languages/properties docs
  * Add notable configuration options (ones affected by this PR's logic changes)
@EricDunsworth
Twitter embedded timeline: Add "verified" account example
49c1177 
Timeline widgets currently work differently between standard and "verified" Twitter accounts:
* Standard accounts: Shows a "Nothing to see here - yet" placeholder message
* "Verified" accounts: Shows top 100 most-liked tweets (Twitter ignores data-tweet-limit attributes)

This adds a second timeline for @canada ("verified" government account) to demonstrate the difference between both "types" of account timelines.

Other changes:
* Use a data-height="fb-page" attribute for the "verified" account example
* Adjust nearby content to be more presentable:
  * Revise example heading (change to plural, update timeline's official name, use sentence case)
  * Revamp Twitter docs link into an expanded section
* Update French translations:
  * Translate main page content
  * Update outdated page description translation
@duboisp
Twitter timeline - Content - Add working example with multiple timelines
22949ce
@duboisp duboisp mentioned this pull request Dec 6, 2023
Merged
@EricDunsworth
Copy link
Owner

@duboisp I'm cool with the way you wrote these extra examples, but am kind of hesitant to include them for other reasons.

Here's why:

  • I'd consider it to be a bad practice to place multiple timeline links within a single plugin container. So I'd prefer not to legitimize that way of coding them in the demo pages.
  • Adding two more "verified" examples would make the demo pages substantially longer, increase loading times somewhat and possibly risk exceeding Twitter's rate limits (although I've personally never run into them myself). Across all 4 examples, 300 tweets would get loaded. I guess what I'm trying to say is that I'd personally prefer to stick with only two examples to keep things as short/simple as possible.

Thoughts?

If you feel strongly about including those extra examples, I suppose I can swallow my ego and merge this PR anyway :D.

I might even tune the plugin to generate mini "inner" containers to make multi-timeline containers as presentable as possible. But I feel like it'd represent a lot of effort for a use case that might not even exist in the wild :S.

@EricDunsworth EricDunsworth force-pushed the v4.0-twitter-add-data-presets-verified-example branch from 23db69c to 91c41ef Compare February 19, 2024 17:52
@EricDunsworth EricDunsworth force-pushed the v4.0-twitter-add-data-presets-verified-example branch 2 times, most recently from f0b31e0 to a0630ab Compare April 18, 2024 19:25
EricDunsworth added a commit that referenced this pull request Apr 22, 2024
@EricDunsworth
Twitter embedded timeline: Add extra examples page
78b2173 
Specifically:
* Added a page with 3 extra examples demonstrating extra scenarios impacted by plugin logic changes:
  * lang="zh-Hans", in context
  * data-dnt="false"
  * data-height="270" (doesn't really tie into logic changes... but isn't demonstrated anywhere else)
* Added links to to the extras page in the main Twitter demo and documentation pages (similar to the form validation plugin's merge server-client errors page)
* Updated modified dates

Inspired by @duboisp (via #2).
EricDunsworth added a commit that referenced this pull request Apr 22, 2024
@EricDunsworth
Twitter embedded timeline: Add extra examples page
362ae4c 
Specifically:
* Added a page with 3 extra examples demonstrating extra scenarios impacted by plugin logic changes:
  * lang="zh-Hans", in context
  * data-dnt="false"
  * data-height="270" (doesn't really tie into logic changes... but isn't demonstrated anywhere else)
* Added links to to the extras page in the main Twitter demo and documentation pages (similar to the form validation plugin's merge server-client errors page)
* Updated modified dates

Inspired by @duboisp (via #2).
Garneauma added a commit to wet-boew/wet-boew that referenced this pull request Apr 23, 2024
@EricDunsworth @Garneauma
Minor - Twitter embedded timeline: Add data-* presets, "verified" exa…
b1a2c37 
…mple, skip links, iframe title fix (#9704)

* Twitter embedded timeline: Add data-* presets

Specifically:
* data-height="500":
  * WET sets it when:
    * Value is explicitly set to "fb-page"
    * data-tweet-limit is used without data-height
  * Needed to prevent older timeline widget implementations from growing extremely tall and to set a reasonable "generic" height going forward
  * Starting July 21, 2023... timeline widgets stopped honouring tweet limits and began showing 100 tweets at a time ("verified" accounts only)
  * 500 pixels matches the default height of the Facebook page plugin (aka Facebook embedded pages)
* data-lang="zh-cn":
  * WET sets it when the in-page language is "zh-Hans"
  * Twitter only supports "zh-cn" for Chinese (Simplified)
* data-dnt="true":
  * Not used if data-lang="false" is already hardcoded

Related changes:
* Plugin now checks every Twitter link in every plugin container (not just the first link)
* Adjust the loading icon's logic accordingly:
  * Move it into the data-* logic's foreach loop
  * Make its initial if condition check for eventTarget.firstElementChild (i.e. only add a loading icon for the first link in the container)
* Documentation:
  * Add specific links to Twitter timelines/languages/properties docs
  * Add notable configuration options (ones affected by this PR's logic changes)

* Twitter embedded timeline: Add "verified" account example

Timeline widgets currently work differently between standard and "verified" Twitter accounts:
* Standard accounts: Shows a "Nothing to see here - yet" placeholder message
* "Verified" accounts: Shows top 100 most-liked tweets (Twitter ignores data-tweet-limit attributes)

This adds a second timeline for @canada ("verified" government account) to demonstrate the difference between both "types" of account timelines.

Other changes:
* Use a data-height="fb-page" attribute for the "verified" account example
* Adjust nearby content to be more presentable:
  * Revise example heading (change to plural, update timeline's official name, use sentence case)
  * Revamp Twitter docs link into an expanded section
* Update French translations:
  * Translate main page content
  * Update outdated page description translation

* Twitter embedded timeline: Add skip to start/end links

* Twitter embedded timeline: Override iframe title

The timeline's iframe title is English-only and written in title case ("Twitter Timeline")...

This replaces it with an i18n version written in sentence case.

* Twitter embedded timeline: Drop limited support for multiple timelines per container

Rationale:
* Looks very strange
* The plugin wasn't designed with it in mind
* Not keen on demonstrating it in official examples
* Skip link logic doesn't play nicely with it
* Traces of support for it in recent logic updates were meant to be "just in case" any implementations were doing it in the wild... but there's no evidence they even exist
* Secondary timelines will still work either way, but won't be "processed" by the plugin

* Twitter embedded timeline: Add invisible start notice

Works much better in screen readers.

Previously, the skip to start link pointed to the timeline iframe's parent div... which caused a ~30-50 second period of silence in NVDA, followed by the entire timeline getting automatically announced.

This addresses it by adding an invisible start notice just before the timeline and changing the skip to start link's destination to go there instead. That causes screen readers to only announce the start notice at first - without delay.

A "fake" outline is also added to the timeline for sighted users (to retain the previous behaviour's visual behaviour - which works better for that audience). The start notice is never visually-revealed to them (showing it as an overlay would've covered part of the timeline... not ideal).

* Twitter embedded timeline: Move skip to end link after start notice

The previous order had the skip link positioned before the start notice.

But it had the following downsides:
* When users activated the skip to start link:
  * Users risked missing out on the skip to end link if they tabbed forward... which might've been problematic if they subsequently got frustrated tabbing through the timeline's endless iframe - without any awareness of the skip to end link's existence.
  * Sighted users would see a "fake" outline on the timeline... but tabbing backwards would focus onto the skip to end link (which is presented as an overlay "inside" the start of the timeline). So the visual order didn't match the focus order.
* Not having both skip links "inside" the timeline (i.e. in-between the start/end notices) arguably betrays user expectations.

Placing the skip to end link after the start notice resolves all of those issues:
* Users will now have better awareness of the skip to end link's presence.
* Sighted users that tab forward will now perceive the skip to end link's overlay as the first element "inside" the timeline.
* Using a sequential order is more in line with user expectations and WCAG 2.x's focus order success criterion (2.4.3).

* Twitter embedded timeline: Update modified dates

* Twitter embedded timeline: Move skip link click handler declaration

Makes more sense to place it in the createSkipLink() function. Especially since createNotice() already does something similar.

Also arguably makes more sense to attach the click handlers directly onto the links themselves (as opposed to the document).

* Twitter embedded timeline: Add workaround for missing iframes in Safari

* Twitter embedded timeline: Polish Safari iframe workaround

Leverage eventTarget (pure JS instead of jQuery) and double-quotes for it.

* Twitter embedded timeline: Update modified dates again

* Twitter embedded timeline: Add extra examples page

Specifically:
* Added a page with 3 extra examples demonstrating extra scenarios impacted by plugin logic changes:
  * lang="zh-Hans", in context
  * data-dnt="false"
  * data-height="270" (doesn't really tie into logic changes... but isn't demonstrated anywhere else)
* Added links to to the extras page in the main Twitter demo and documentation pages (similar to the form validation plugin's merge server-client errors page)
* Updated modified dates

Inspired by @duboisp (via EricDunsworth#2).

* Twitter embedded Timeline: Use XDevelopers for extra 2nd example

Avoid having redundant sets of Canada timelines and associated skip links.

The plugin doesn't currently attempt to inject extra content to distinguish redundant timelines.

* Twitter embedded Timeline: Remove extra's trailing spaces

* Twitter embedded Timeline: Update modified dates yet again

---------

Co-authored-by: Marc-André Garneau <[email protected]>
@EricDunsworth
Copy link
Owner

Gonna close this out since wet-boew#9704 has now been merged.

@duboisp Btw thanks for sending this :)! Even though I didn't ultimately merge it, IMO it had a positive impact. It caused me to decide to drop "unofficial" multi-timeline support and introduce the extra examples page to show more varied examples influenced by plugin logic changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@duboisp @EricDunsworth