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

Rulers #5750

Merged
merged 3 commits into from Jan 11, 2019
Merged

Rulers #5750

merged 3 commits into from Jan 11, 2019

Conversation

@fcollonval
Copy link
Member

@fcollonval fcollonval commented Dec 10, 2018

Fixes #4179 by building on #4294

Styling of the rulers is done through a class jp-CodeMirror-ruler which is set to

.jp-CodeMirror-ruler {
  border-left: 1px dashed var(--jp-border-color2);
}

Screenshots
image

@fcollonval
Copy link
Member Author

@fcollonval fcollonval commented Dec 10, 2018

A thought: would it be better to set the all border style (1px dashed var(--jp-border-color2);) as one CSS variable to ease styling through theme?
If so, where is the best place to store it?

Copy link
Member

@ian-r-rose ian-r-rose left a comment

Thanks for taking this over @fcollonval (and sorry for the slow reply!). This looks great to me. I like your approach to styling. My main concern is that I am not sure we should be enabling rulers by default, since it is a personal choice for a lot of people. Can we make the default value an empty array [] instead of [88]?

@ian-r-rose ian-r-rose mentioned this pull request Jan 8, 2019
@fcollonval
Copy link
Member Author

@fcollonval fcollonval commented Jan 11, 2019

Thanks for the review @ian-r-rose
I update the PR according to your review. Should I rebase the PR before you can merge it?

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Jan 11, 2019

Thanks @fcollonval, this looks great. No need to rebase unless there are conflicts (which there do not appear to be here, but may occur in the code folding PR :))

@ian-r-rose ian-r-rose merged commit c7ba12f into jupyterlab:master Jan 11, 2019
2 of 3 checks passed
@jasongrout jasongrout added this to the 1.0 milestone Feb 2, 2019
@fcollonval fcollonval deleted the rulers branch Apr 30, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants