Code Syntax Highlighting (take 2)

[felixcheung]

#300 - continuing from where we left off. I ran into some very bad error with js and couldn't figure out how to make it not error out, so I finally gave up I started clean in a new branch.

This PR includes Markdown code block syntax highlighting and Ace Editor theme setting, as an example. It only set 1 theme (matching for now) for each.

Also detailing in documentation what's required if someone wants to change it.

Here's what it looks like:

And here's 'what-if' if we are to change theme for the editor (but not markdown) This is not checked in. To be clear, no way to use this theme without changing code/build)

Feedback appreciated.

[felixcheung]

@corneadoug @Leemoonsoo @swkimme @anthonycorbacho ?

[swkimme]

Reviewed, much shorter and cleaner codes than before! Great!!
Let me give some time to test this PR.
Great job!

[Leemoonsoo]

While testing, i've found one problem with block selection in code editor.

[felixcheung]

Thanks, I noticed that too when I switched to a different editor theme, didn't realize it was happening somewhere else.
A stylesheet is overriding the foreground color it seems. Let me investigate.

[swkimme]

Adding transparent background to hljs class would be cooler.

[felixcheung]

Something is overriding the selection but it's not

app/style/notebook.css

.ace-tm .ace_marker-layer .ace_selection {
    background: rgba(98, 157, 233, 0.2) !important;
    color: black;
}

.ace_active-line {
    background: rgba(98, 157, 233, 0.2) !important;
}

.ace-tm .ace_marker-layer .ace_selected-word {
    background: rgba(250, 250, 255, 0.2) !important;
}

any idea?

[felixcheung]

Fixed editor selection color issue.
@swkimme +1 for gray though :)

[felixcheung]

Any more thoughts or comments?

[Leemoonsoo]

I still have selection problem, am i missing something?

[felixcheung]

@Leemoonsoo Sorry about that, I must have been messing it too much in the browser dev tool. That was only a partial fix.

Here's what it looks like now:

And there has been a similar issue with word selection and it is also fixed:

[Leemoonsoo]

Selection now works well!

There is a box in a code block and have slightly different background color, ie

Will this PR handle the issue, too?

[felixcheung]

@Leemoonsoo Good point. This was also pointed out in PR #300. Since we bring in both bootstrap and highlight.js with bower, I'm not sure which is the best option to address this:

1. Override markdown code block box background in app/style/notebook.css
2. Fork/create a new stylesheet file and change code block style background in the css

Option 1 is rather hard to discover for a new comer (me?) to find all those overrides....

Also, we have options on the color to change it to:
a. #f5f5f5
b. transparent (therefore just as before, as suggested by @swkimme)

Could we merge this for now and I'll follow up with another PR?

[anthonycorbacho]

why another PR?
this is the perfect PR for fixing the issue.

[corneadoug]

I wanted to just push the CSS fix, but it means I would nee to fork @felixcheung Zeppelin fork, then do a pull request on his fork.....

So, since there is no theme switch for hightlight.js, and we don't handle css theme change, we can just add the fix to notebook.css.

.paragraph .tableDisplay .hljs {
  background: none;
}

.paragraph .ace_print-margin {
  background: none !important;
}

This also fix an annoying margin border on Ace editor

Before

After

[swkimme]

+1 for @corneadoug 's solution

[Leemoonsoo]

+1 for @corneadoug 's solution

[felixcheung]

Updated.

[Leemoonsoo]

Tested and Looks good to me

[Leemoonsoo]

I added this PR to https://github.com/NFLabs/zeppelin/issues/383.
I'm merging it if there're no more discussions!

[Leemoonsoo]

@felixcheung Thanks for contribution! awesome job!

[felixcheung]

Very Nice! Thanks

[swkimme]

Great job!

On 2015년 3월 19일 (목) 12:53 Felix Cheung notifications@github.com wrote:

Very Nice! Thanks

—
Reply to this email directly or view it on GitHub
#379 (comment).