Bugzilla – Full Text Bug Listing
| Summary: | Improve styling of code elements (for benefit of README.md backtick (code) markup) | ||
|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Adrian Aichner <adrian.aichner> |
| Component: | Client | Assignee: | Project Inbox <orion.client-inbox> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | adrian.aichner, mamacdon, Silenio_Quarti |
| Version: | unspecified | Flags: | Silenio_Quarti:
review+
|
| Target Milestone: | 5.0 M1 | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Whiteboard: | |||
This enhancement is ready for merge at https://github.com/anaran/orion.client/commit/29bd50847d065b5c8221f8e308700f5b40c0f9e0 Let me know if need to do anything else to get this resolved. Thanks! Adrian I assert that I authored 100% of the content of this contribution and have the rights to donate the content to Eclipse under the EPL Thanks for the patch. FYI, Eclipse has updated its contribution process. Before we can merge your commits, you will have to sign a CLA by logging in here and completing the Contributor License Agreement form: https://projects.eclipse.org/user/login/sso The good news is, after you've signed the CLA you never have to post an "I assert that I authored..." declaration ever again :) Hi Mark, I now have a green checkmark in my CLA box! Turns out I produced an incorrect solution. I was fooled by the fact that "```" seems to be treated just the same as "`" in orion origin/master at the moment. org.eclipse.orion.client.ui/web/marked/marked.js says it supports GFM by default, but then I would expect ```CONTENT``` to produce <pre>CONTENT</pre>, not <code>CONTENT<code> (just like `CONTENT` does). What am I missing? (In reply to Adrian Aichner from comment #1) > This enhancement is ready for merge at > > https://github.com/anaran/orion.client/commit/ > 29bd50847d065b5c8221f8e308700f5b40c0f9e0 > > Let me know if need to do anything else to get this resolved. > > Thanks! > > Adrian Is it a goal to produce the same HTML markup structure from GFM markdown as github currently does? If not, then I have an updated version of my patch ready for pulling: https://github.com/anaran/orion.client/commit/661c2f992bcd85a19617ea7287ca94155df7f463 Do you want me to send a github pull requests as well? (I have not done so since I was afraid it may cause confusing) Adrian (In reply to Adrian Aichner from comment #5) > Turns out I produced an incorrect solution. > > I was fooled by the fact that "```" seems to be treated just the same as "`" > in orion origin/master at the moment. > > org.eclipse.orion.client.ui/web/marked/marked.js says it supports GFM by > default, but then I would expect ```CONTENT``` to produce > <pre>CONTENT</pre>, not <code>CONTENT<code> (just like `CONTENT` does). > > What am I missing? > > (In reply to Adrian Aichner from comment #1) > > This enhancement is ready for merge at > > > > https://github.com/anaran/orion.client/commit/ > > 29bd50847d065b5c8221f8e308700f5b40c0f9e0 > > > > Let me know if need to do anything else to get this resolved. > > > > Thanks! > > > > Adrian Comment 6 is ready to be pulled from my end. Thanks, Adrian (In reply to Adrian Aichner from comment #6) A pull request on GitHub is not necessary -- we can fetch the commits from your remote. (If you want to directly request a review in the future, you can push directly to Gerrit [1] if you wish.) [1] http://wiki.eclipse.org/Orion/How_Tos/Using_Gerrit_in_Orion (In reply to Adrian Aichner from comment #5) > org.eclipse.orion.client.ui/web/marked/marked.js says it supports GFM by > default, but then I would expect ```CONTENT``` to produce > <pre>CONTENT</pre>, not <code>CONTENT<code> (just like `CONTENT` does). > > What am I missing? I'm not sure. When I run a fenced GFM code block through Marked, e.g. ``` CONTENT ``` I get <pre><code>CONTENT</code></pre>, which matches GitHub's output. (Were you using "```" inline? In that case it's not treated as a fenced block, so you just get a <code> with no <pre>. And again, I see the same output from Marked as GitHub.) I reviewed the patch from Comment 6, and added some comments [2]. Once they are addressed, I will +1 this. [2] https://github.com/anaran/orion.client/commit/661c2f992bcd85a19617ea7287ca94155df7f463 (In reply to Mark Macdonald from comment #8) I just saw this commit go by which styles the .orionMarkdown pre elements, so some of my comments are probably stale now. I imagine the patch will need to be rebased/modified as well... http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=30f0455 Hi Mark, thanks for your review. Is the patch referenced below OK as is? I will rebase and see whether I have any remaining issues from my end. (In reply to Mark Macdonald from comment #9) > (In reply to Mark Macdonald from comment #8) > > I just saw this commit go by which styles the .orionMarkdown pre elements, > so some of my comments are probably stale now. I imagine the patch will need > to be rebased/modified as well... > > http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/ > ?id=30f0455 (In reply to Adrian Aichner from comment #10) > Hi Mark, thanks for your review. > Is the patch referenced below OK as is? > I will rebase and see whether I have any remaining issues from my end. Well, I just tried out master, and fenced blocks are now taken care of. However we're still lacking a border on inline code blocks -- which I guess would be ".orionMarkdown p>code". That would be an opportunity if you're interested in reworking your patch.. (In reply to Mark Macdonald from comment #11) > (In reply to Adrian Aichner from comment #10) > > Hi Mark, thanks for your review. > > Is the patch referenced below OK as is? > > I will rebase and see whether I have any remaining issues from my end. > > Well, I just tried out master, and fenced blocks are now taken care of. > However we're still lacking a border on inline code blocks -- which I guess > would be ".orionMarkdown p>code". That would be an opportunity if you're > interested in reworking your patch.. (already confirmed by you: I rebased and .markDown pre looks good. `CODE` does not get a border.) I guess some of the properties from .orionMarkdown pre { background-color: #fafafa; border: 1px solid #ddd; padding-top: 10px; padding-bottom: 10px; overflow: auto; } should be pulled down into a new .orionMarkdown code { I wonder if it would be better for you and Silenio to work this out directly? I don't know enough about the overall design you have in mind the the markdown styling. Just let me know... (In reply to Adrian Aichner from comment #12)> > I guess some of the properties from > > .orionMarkdown pre { > background-color: #fafafa; > border: 1px solid #ddd; > padding-top: 10px; > padding-bottom: 10px; > overflow: auto; > } > > should be pulled down into a new > > .orionMarkdown code { I think that makes sense to me. How can I get the mark down to generate a 'code' element not wrapped by a 'pre' element? (In reply to Silenio Quarti from comment #13) > (In reply to Adrian Aichner from comment #12)> > > I guess some of the properties from > > > > .orionMarkdown pre { > > background-color: #fafafa; > > border: 1px solid #ddd; > > padding-top: 10px; > > padding-bottom: 10px; > > overflow: auto; > > } > > > > should be pulled down into a new > > > > .orionMarkdown code { > > I think that makes sense to me. How can I get the mark down to generate a > 'code' element not wrapped by a 'pre' element? Hi Silenio, both `CODE` and ``CODE`` will generate <p> <code>DOUBLE</code> </p> That's what I had used in my testng. Thanks for looking into this! Pushed. code and pre share some properties, but code properties need to be overwritten when code is inside a pre element, otherwise we get double borders. http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=2ae6011593166daa1945f1de24b0f99f27c52401 Please try the latest. Thanks! (In reply to Silenio Quarti from comment #15) > Pushed. code and pre share some properties, but code properties need to be > overwritten when code is inside a pre element, otherwise we get double > borders. > > http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/ > ?id=2ae6011593166daa1945f1de24b0f99f27c52401 > > Please try the latest. Thanks! Looks pretty good, but I would add some horizontal padding as well. E.g. github uses 5px padding horizontally for p>code and 10px for pre>code. Looks more pleasing, especially in combination with the generous vertical padding. How about this? .orionMarkdown pre { padding: 6px 10px; +.orionMarkdown pre code { padding: 0 5px; I believe with this commit we match github now. http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=dbee6f54d5f978364eb9ce80a2a12af004c7dead (In reply to Silenio Quarti from comment #17) > I believe with this commit we match github now. > > http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/ > ?id=dbee6f54d5f978364eb9ce80a2a12af004c7dead I like it! Works well for me on my local rebased orion server running origin/master! Thanks, Silenio, I won't reopen this one :-) Thanks for confirming. |
Adding code { border: 1px solid; display: block; } to bundles/org.eclipse.orion.client.ui/web/css/ide.css makes code blocks stand out nicely, not entirely unlike the styling of github markdown documents. I will add a comment to this bug when I have a branch or patch ready for this small enhancement.