Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Replace innerText call with textContent #1422

Conversation

Blackbaud-StacyCarlos
Copy link
Contributor

@Blackbaud-StacyCarlos Blackbaud-StacyCarlos commented Jan 15, 2018

A bug was encountered when testing a directive within a SkyUX2 SPA where overflow styling was causing certain test to fail. This was determined to be caused by the use of the innerText element property.

The innerText property is only text that is actually rendered on the page while textContent is the all text, regardless whether it is rendered.

Because the point of unit tests is to test the TypeScript portion of the application and the visual tests determine if the content is correctly rendered, I think this should be pulled to master.

@blackbaud-sky-savage
Copy link
Collaborator

Tests passed. Automated cross-browser testing via BrowserStack and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED

Commit: f06dd3c
Build details: https://travis-ci.org/blackbaud-sky-savage/skyux2/builds/328904833

(Please note that this is a fully automated comment.)

@codecov-io
Copy link

codecov-io commented Jan 15, 2018

Codecov Report

Merging #1422 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1422   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         370     370           
  Lines        6789    6789           
  Branches      874     874           
======================================
  Hits         6789    6789

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db5852d...f06dd3c. Read the comment docs.

Copy link
Member

@Blackbaud-SteveBrush Blackbaud-SteveBrush left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants