Twitter embedded timeline: Improve loading phase

[EricDunsworth]

The plugin's loading icon was previously tied to the "twitter-timeline" class... which is used by both the Twitter link and timeline container.

That resulted in the following behaviour:

1. The link's loading icon immediately appeared
2. A second loading icon appeared when Twitter's widget script injected an empty timeline container into the page
3. The link's loading icon got removed when the timeline finished loading... leaving the timeline's loading icon in place forever

The following issues also occurred:

• The empty space to the right of the link was a linked area
• The link's loading icon was a linked area
• The link's loading icon appeared "on top" of the link in mobile viewports
• The loading icon was center-aligned in a full-width overlay... causing it to look like it was in the middle of nowhere in larger viewports
• The link's loading icon appeared forever in IE11

This fixes it by reworking the loading phase as follows:

• The link's linked area now only covers the link text
• The plugin now injects a loading icon div below the link
• The loading icon now...
  • Appears below the link instead of "on top" of it
  • Is either left/right-aligned or center-aligned relative to the link if a browser supports the :has() pseudo-class
  • Disappears when Twitter's timeline widget removes the link (CSS immediately hides it, then the plugin removes it behind-the-scenes)
  • Never appears in IE11

Other:

• Only shows one loading icon per plugin container
• Plugin containers with extra Twitter links or non-standard structures won't show loading icons
• Disables the plugin in IE11 (Twitter's widget script no longer supports that browser)

Related:

• Fixes [BUG] Twitter's spinning icon still shows #9503
• Depends on Build: Replace csslint with stylelint #9539

Screenshots...

Before:

• Phase 1 (loading icon on top of the link... centered relative to viewport)
  
• Phase 2 (two loading icons on top of the link)
  
• Phase 3 (centered loading icon on top of the timeline... forever)
  

After:

• Phase 1 (link before plugin init)
  
• Phase 2 (loading icon below the link... left-aligned by default, centered relative to link if :has() is supported)
  OR
• Phase 3 (timeline)
  

[EricDunsworth]

The build failures are being caused by CSSLint lacking support for :has() and nested calc() expressions.

Since CSSLint has gone stale (latest release was in fall 2016), it'll probably be necessary to migrate WET to a more modern CSS linter. I'm gonna try looking into stylelint.

On a random side note, the reason :has() didn't trigger any build failures in wet-boew/GCWeb#1916 is because GCWeb doesn't currently use any CSS linters at all :S. CSSLint was probably removed from it when it adopted Jekyll.

[duboisp]

Pre-approved upon successful testing and after the sasslint exception is added for both selector or the file if not possible.

[EricDunsworth]

Just tried adding inline CSSLint ignore comments (/* csslint ignore:start */ and /* csslint ignore:stop */) without any luck :(.

From what I could surmise from issue discussions in CSSLint/csslint, it looks like its ignore syntax isn't unconditional. Therefore, the things it dislikes about my :has() and calc() syntax are probably impossible to ignore.

Btw its ignore syntax isn't formally documented anywhere :S. Closest I found was a wiki link to a non-existent page.

As far as ignoring src/plugins/twitter/_base.scss in Gruntfile.coffee goes, I don't think that'll be possible either since CSSLint is used to lint compiled CSS files. So my only quick-fix solution would to exclude dist/theme-wet-boew/css/theme.css and dist/theme-wet-boew/css/ie8-theme.css from CSSLint... which IMO is a non-starter since it'd effectively exclude almost all of WET's CSS.

Pre-approved upon successful testing and after the sasslint exception is added for both selector or the file if not possible.

@duboisp Just to clarify - SASSLint lints SCSS source files and has no trouble with this PR. CSSLint lints compiled CSS and is causing my hurdles. I think migrating to stylelint will be my only viable option to get out of this mess. So this PR will probably need to be held back until next week.

[duboisp]

@EricDunsworth humm, I think it's /* csslint ignore:end */ not "stop", see: https://github.com/CSSLint/csslint/blob/master/src/core/CSSLint.js#L211-L225

It need both in order to work.

[EricDunsworth]

@duboisp Just tried /* csslint ignore:start */ and /* csslint ignore:end */ without any luck :(.

Btw good catch on the end syntax. I stumbled upon stop via CSSLint/csslint#564 while searching around the repo. Guess it got renamed to end later on.

[duboisp]

See the build error, meanwhile I am continuing my local testing.

[EricDunsworth]

Just force-pushed an update :)!

Here's what's changed:

• Revise IE11 ignore logic to disable the entire plugin in that browser
• Rework logic to treat each plugin instance as having only one Twitter link situated at the beginning (i.e. strictly adhere to WET's demo/documentation structure)
  • Plugin containers with extra links or non-standard structures will still load all their timelines, but won't show loading icons
• Only use the mutation observer if a loading icon gets inserted

Heads up:
@duboisp We're probably going to need to discuss variable declarations and const some more.

At one point earlier today, I had them setup like what the requested changes wanted and marked most as resolved. But then I significantly reworked more logic and didn't feel like that way of declaring variables made sense anymore. So I spread-out the declarations and went back to const for loadingDiv.

I think we're gonna need to "pick our poison" 😛:

1. Status quo (scoping out variables)
2. Readability (grouping all variables at the start of the plugin
3. Hybrid (move scoped variable declarations around so they come right after the comments for the logic they tie into):
   • Arguably the best of both worlds
   • Would improve scoping since it'd allow the mutation observer to be declared as const (it's currently using let since I wanted to keep scoped variable declarations at the beginning of their code blocks)
   • Readability would be improved since scoped variable declarations would always be "in context" (i.e. directly after the comments for the logic they're needed for)
   • Downside is that this approach would betray common programming conventions (i.e. everyone usually expects to see variable declarations at the beginning of functions or code blocks)

Thoughts?

[GormFrank]

Pre-approved upon successful review & testing

[duboisp]

Reviewed and tested locally (in FF and Edge) and work as expected.

Thanks @EricDunsworth