-
Notifications
You must be signed in to change notification settings - Fork 169
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
Show search notebook link in handbook and blog #4899
Conversation
Notifying subscribers in CODENOTIFY files for diff 9314328...74a62a7.
|
Cc @benvenker @lguychard not sure what you all think of this. It's very hacky. :) |
@@ -276,3 +276,8 @@ button#CybotCookiebotDialogBodyButtonDetails { | |||
.container-md { | |||
max-width: 1140px; | |||
} | |||
|
|||
/* Fix so that turning off outer pre does not impact styling */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps not really necessary? There's just a bit of extra padding around syntax highlighting without it, that isn't there on about.sourcegraph.com today because of re-adding the wrapping pre
.
@@ -0,0 +1,31 @@ | |||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should be deleted before merging.
@@ -8,3 +8,5 @@ require ( | |||
github.com/sourcegraph/docsite v1.6.0 // indirect | |||
golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d | |||
) | |||
|
|||
replace github.com/sourcegraph/docsite => github.com/jyavorska/docsite v1.8.3-0.20210930071952-c6a6c94c17fe |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line should be deleted (and the docsite version incremented) if github.com/sourcegraph/docsite is updated to stop supressing the wrapping pre
.
@@ -8,6 +8,10 @@ The Product team strives to make the following true: | |||
- Each teammate has the customer and product context needed (about customer problems, likely future priorities, possible solutions, etc.) to perform their work effectively. | |||
- The product vision and roadmap are communicated well to teammates and everyone outside Sourcegraph. | |||
|
|||
## Test page, please remove |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete this section before merging.
@@ -0,0 +1,9 @@ | |||
# Test page |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete this file before merging.
def hello | ||
``` | ||
|
||
<script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be moved to some reused place.
@jyavorska great idea! I think that, unfortunately, the current implementation is likely to be broken by On the flipside, when we get a NextJS-backed handbook, we also get mdx support, which means we can embed React components within Markdown files. Thus, we can more easily render notebooks component directly in the handbook instead of just linking to Sourcegraph! |
This is awesome! (Feels like one of those obvious "of course we have this" things that we don't yet have!) I'll defer to everyone else on the outcome re: the handbook migration, but @jyavorska just wanted to note that the Sourcegraph docs use I only just learned about ```sgquery two weeks ago, and I don't think even people who've been here many years longer than me are aware of it since they also learned about it then, so unifying them might help build awareness. |
@
Thanks @Joelkw! I based off that recent VS Code article, and they used |
This PR is not mergeable due to the handbook refactor, closing. |
@csbailey5t Is this what you were hoping we would be able to do (with the Paul Jolly/CUE post)? |
This is the first step of that. The goal would be being able to run the search notebook code blogs on the blog page, rather than linking out to the notebook on Sourcegraph. |
This makes it so if you include a Sourcegraph notebook code fence in your markdown, it will add a link to the search page. For this iteration, it does not try to render a control to embed a working search on the page, just makes it so you can get to the search easily. Styling is absolutely minimal just to prove it works.
For example, using the following in markdown:
Will render as the following in the handbook:
Or as follows on the site:
Notes
github.com/sourcegraph/docsite
to re-enable the wrappingpre
, since otherwise no CSS classes indicating the language that was being rendered was available, and this required a minor CSS addition to ensure it rendered similarly to how it looks today. See https://github.com/jyavorska/sourcegraph for the fork, and layout.css for the handbook CSS adjustment. There's also an adjustment in thego.mod
in this repo to replace the official docsite repo with my fork that needs to be removed if/when the main sourcegraph docsite repo is updated.