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

Patch out bootstrap rule forcing black and white printing #3212

Merged
merged 1 commit into from Jan 22, 2018

Conversation

Projects
None yet
3 participants
@takluyver
Copy link
Member

takluyver commented Jan 15, 2018

A thoroughly ugly hack. Bootstrap forces all text to be black when printing, resulting in a long running issue (#840). I tried to override it with color: inherit !important, but that didn't produce the right results. I can't find any way to tell LESS to remove a rule or act as if it didn't exist. So the only thing I can think to do is to patch the source we get from bootstrap to get rid of the rule entirely.

Frustratingly, it looks like bootstrap is fixing this (getting rid of the rule) for v4, but it's also moving from LESS to SCSS, so we can't easily switch to the new version.

Closes gh-840

@takluyver

This comment has been minimized.

Copy link
Member Author

takluyver commented Jan 15, 2018

The test failures on Travis appear to be genuine failures from nbval. I just released 0.8, which catches some discrepancies that earlier versions of nbval missed. CC @vidartf .

@Carreau
Copy link
Contributor

Carreau left a comment

It's been long standing enough. +1 for hackish solution.

@damianavila
Copy link
Member

damianavila left a comment

It is ugly but it is a long standing issue and doing it less hacky way implies a lot of additional complexity.

+1 as well.

rmed = lines.pop(ix)
print("Removed line", ix, "from bootstrap print.less:")
print("-", rmed)
print()

This comment has been minimized.

@damianavila

damianavila Jan 21, 2018

Member

why this empty print here?

This comment has been minimized.

@takluyver

takluyver Jan 21, 2018

Author Member

It makes a blank line, so the text above is slightly easier to pick out.

This comment has been minimized.

@damianavila

damianavila Jan 22, 2018

Member

Fair enough 👍

lines = f.readlines()

for ix, line in enumerate(lines):
if 'Black prints faster' in line:

This comment has been minimized.

@damianavila

damianavila Jan 21, 2018

Member

I guess this is enough specific to patch the proper line, right?

This comment has been minimized.

@takluyver

takluyver Jan 21, 2018

Author Member

Should be. The original file is not very long:

https://github.com/twbs/bootstrap/blob/v3.3.7/less/print.less

@Carreau Carreau merged commit 0254c8b into jupyter:master Jan 22, 2018

4 checks passed

codecov/patch Coverage not affected when comparing de92a2b...622a886
Details
codecov/project 78.73% remains the same compared to de92a2b
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Carreau

This comment has been minimized.

Copy link
Contributor

Carreau commented Jan 22, 2018

Thanks both !

@damianavila

This comment has been minimized.

Copy link
Member

damianavila commented Jan 31, 2018

@mpacer @takluyver, we need to update the css in the CDN so nbconvert can catch this change.
Will open an issue to track this need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.