Terra-core: [terra-button] [terra-list] Resolve WDIO screenshot hover tests

[neilpfeiffer]

Bug Report

Description

Created from this comment: https://github.com/cerner/terra-core/pull/2929#issuecomment-620058376

Need to investigate why byproduct of this fix #2929 does not work for screenshot tests - such that the hover tests no longer show the hover styles applied as intended by the tests. Fix could be to create an alternate test method or investigation why headless chrome doesn't capture hover style for test like this ../wdio/button-spec.js#L10-L19

Looks like it also is a problem in list, where this fix was first applied. Although -- not sure why, it didn't seem to cause an update for screenshots when it was first applied on Apr 10, 2019 on PR #2332 -- instead, it wasn't until Dec 2, 2019 that the screenshots changed.

Steps to Reproduce

1. Apply media styles similar to pull/2929/files#diff-c2c3bae...R18
2. Run screenshots tests
3. Screenshots fail unless updated with hover-style no longer visible.
   Diff Example: ...__snapshots__/reference/en/chrome_tiny/button-spec/Neutral-Hovered[default].png

Additional Context / Screenshots

• Emphasis-Hovered[default].png
• Ghost-Hovered[default].png
• Neutral-Hovered[default].png
• De-emphasis-Hovered[default].png

Expected Behavior

WDIO tests for hover style should correctly capture the hover appearance styling in screenshots.

Notes

Fixes will also need to be added to https://github.com/cerner/terra-core/pull/2932 and any other companion similar issues in other repos.

[emilyrohrbough]

This is an issue when the following media query is used for hover:

@media screen and (-ms-high-contrast: active), (-ms-high-contrast: none), (hover: hover) and (pointer: fine) {
  &:hover {
      // styles
  }
}

[supreethmr]

Instead of depending on media queries to remove hover styles on Touch- devices. We can use ontouchstart value to check whether the device is touch device or not. Depending in this value we can restrict hover-styles for touch devices by using :not() function of css like below:

    &:hover:not(.is-touch-device)

// before render set value for is-touch-device
   { 'is-touch-device': ('ontouchstart' in window) },

[emilyrohrbough]

@supreethmr We discussed this proposal. Are there any other, non-javascript options that will resolve this issue?

// cc @neilpfeiffer

[supreethmr]

Reason for considering ontouchstart event handler for fixing hover styles for touch-devices are :

1. ontouchstart is being used in terra-components like terra-form-select so I don't think there will be any compatibility issues with this approach.

2. The code changes are minimal for using ontouchstart to restrict hover styles on touch devices.

Disadvantages with using @media queries are :

1. There are no standard media query available to support hover media feature in IE. -ms-high-contrast is marked as non-standard feature and might be changed in future releases of Microsoft. Reference : https://developer.mozilla.org/en-US/docs/Web/CSS/@media/-ms-high-contrast

2. Since our WDIO uses headless chrome for visual regression testing these hover style rule will apply for all tests irrespective of the viewport size. It seems that headless chrome doesn't support the @media (hover: hover) media query, could be similar to issue : (https://github.com/puppeteer/puppeteer/issues/4820). Due to this issue hover styles in WDIO tests are missing.

And Other CSS options like Pointer CSS media feature will also not work with headless chrome.

I think the other option would be to identify the touch devices using screen width in media queries. But I am not sure of the limitations with this approach.

[neilpfeiffer]

Thank you for the investigation and documentaiton @supreethmr -- for reference, channelling my inner @bjankord and some background as to why we do not feel comfortable using the ontouchstart method as the solve.

We have very intentionally made it a general rule that Terra does not attempt to do user-agent, device, event, or feature detection within our JavaScript, because it can lead to unreliable confidence in being able to customize Terra applications for mobile, and falsely assumes user behavior based on device capability. This becomes dangerously close to an approach like Modernizr, where initialization detections are made and global classes are applied to base/body/html to apply to the full application -- which Terra has also intentionally rejected such methods in the past as it runs contrary to being browser/device agnostic.

(Yes, there are however a very small number of places where we have made special exceptions, usually _only_ after much discussion, and agreement to limited usage until alternate methods can replace them.)

Having the complex media queries is not perfectly ideal either, but less concerning as an approach mainly since it is exclusively CSS. Agree that the current 4-part query conflicts with WDIO, so not perfect here either.

Please hold @saket2403 - we are going to have a discussion about approaches, since there is a secondary issue also being discussed for mobile apps that might be related in the solve, depending on how it's decided to handle one or both of these.

[neilpfeiffer]

@saket2403 The updated direction for
cerner/terra-core#2932
https://github.com/cerner/terra-clinical/pull/652
https://github.com/cerner/terra-framework/pull/1095

• [ ] revert and remove the ontouchstart detection and the 'is-touch-device' class, and return back to the compound media query @media screen and (-ms-high-contrast: active), (-ms-high-contrast: none), (hover: hover) and (pointer: fine) { &:hover { ... } }.

• [ ] since this prevents webdriver's ability to capture the hover visual style, we can reasonably shift to manual unscripted testing for all hover testing, so for all packages we are applying this media query, we can remove any corresponding hover wdio tests, plus remove the corresponding snapshots and test examples.