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

feat(notebook): add one-dark theme colors #1756

Merged
merged 1 commit into from Jun 27, 2017

Conversation

Madhu94
Copy link
Contributor

@Madhu94 Madhu94 commented Jun 25, 2017

No description provided.

@Madhu94 Madhu94 closed this Jun 25, 2017
@codecov-io
Copy link

codecov-io commented Jun 25, 2017

Codecov Report

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

@@           Coverage Diff           @@
##           master    #1756   +/-   ##
=======================================
  Coverage   78.43%   78.43%           
=======================================
  Files          62       62           
  Lines        2091     2091           
=======================================
  Hits         1640     1640           
  Misses        451      451

@Madhu94 Madhu94 reopened this Jun 26, 2017
body
{
line-height: 1.3 !important;
}
Copy link
Member

Choose a reason for hiding this comment

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

Where does this one come from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rgbkrk Uhm..The other themes had this property set, so I just let it stay here too.

Copy link
Member

Choose a reason for hiding this comment

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

Oh hmmmm, perhaps we need that in main.css then?

Copy link
Contributor Author

@Madhu94 Madhu94 Jun 27, 2017

Choose a reason for hiding this comment

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

@rgbkrk That's right, I should have done that. Guess I should learn to step back and look at the big picture, instead of focusing narrowly on the specific problem I was trying to solve :-(

The line-height and the webkit scrollbar style properties are in all themes except the halloween, christmas and the "undefined" theme, which are not even exposed through the UI right now. I will move them into main.css very soon.

What is this undefined theme, anyway?

Copy link
Member

@rgbkrk rgbkrk Jun 27, 2017

Choose a reason for hiding this comment

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

Those are all easter eggs. 😄

⚠️ the undefined theme may not be safe for people sensitive to color flickering. It was put in place to notice a certain class of theme related bugs for humor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

:-) Easter eggs should be exposed to the end-user somehow, no? These themes are exposed to the developer but...

Copy link
Member

Choose a reason for hiding this comment

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

The Christmas and Halloween ones only come up during certain times of the year (and it would be ok if we had more). undefined has only one way of coming up, though you can enable it using the ~/.jupyter/nteract.json file. 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"during certain times of the year " -> Gonna hunt for this part of the code.

" it would be ok if we had more" -> I'll add an Indian holiday soon if I can come up with the colors 😃

Copy link
Member

Choose a reason for hiding this comment

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

I'll add an Indian holiday soon if I can come up with the colors

That's awesome!

By the way, I'm trying out packaging of the app again to prep for another release.

Copy link
Member

Choose a reason for hiding this comment

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

I'll add an Indian holiday soon if I can come up with the colors

@Madhu94 that would be awesome!

By the way, I'm trying out packaging of the app again to prep for another release.

@rgbkrk I messed around with nteract some over the weekend. I wasnt able to build the deb file on ubuntu, but figured it was something I needed on my system. (I can get dev mode to work fine)

@rgbkrk rgbkrk merged commit 431550a into nteract:master Jun 27, 2017
@rgbkrk
Copy link
Member

rgbkrk commented Jun 27, 2017

Thanks for doing this!

@Madhu94 Madhu94 mentioned this pull request Jun 29, 2017
@lock
Copy link

lock bot commented Apr 2, 2018

This thread has been automatically locked because it has not had recent activity. Please open a new issue for related bugs and link to relevant comments in this thread.

@lock lock bot locked and limited conversation to collaborators Apr 2, 2018
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.

None yet

4 participants