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.

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

Revamp linting, fix bug found from linting #154

Merged
merged 2 commits into from
Aug 6, 2019
Merged

Revamp linting, fix bug found from linting #154

merged 2 commits into from
Aug 6, 2019

Conversation

MatthewHerbst
Copy link
Owner

This fixes our linting such that npm run lint now works, and from that it fixes a bug that was found with linting working:

When we encountered a link that was not a STYLE tag and that had an href value, we would override the loop going through all the stylesheets with an inner loop, possibly causing us to miss some style sheets or to attempt to access stylesheets that did did not exist on the page.

When we encountered a link that was not a STYLE tag and that had an
`href` value, we would override the loop going through all the
stylesheets with an inner loop, possibly causing us to miss some
style sheets or to attempt to access stylesheets that did did not
exist on the page.
Copy link

@davegw davegw left a comment

Choose a reason for hiding this comment

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

LGTM 👍

src/index.tsx Outdated Show resolved Hide resolved
This allows for not needing to compare against `length - 1`
@MatthewHerbst MatthewHerbst merged commit 332f3fa into MatthewHerbst:master Aug 6, 2019
@MatthewHerbst MatthewHerbst deleted the lint-revamp+bug-fix branch August 6, 2019 22:09
@MatthewHerbst
Copy link
Owner Author

I accidentally include another bug fix in this PR.

We were seeing this error: TypeError: Object doesn't support property or method 'remove'

The fix is to remove from the parent node, rather than trying to remove the node directly. We already use this method for removing the iframe if it already exists when trying to print a second time.

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

Successfully merging this pull request may close these issues.

2 participants