From 59d7937a1be9ed88b9d851b88976224268a71e0c Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Mon, 26 Feb 2024 18:40:56 +0100 Subject: [PATCH 1/5] feat: add CSS classes for code cells in in wrapper.go This removes the need for custom renderers to do that on their own --- render/html/html.go | 6 ------ render/html/html_test.go | 23 ++++------------------- render/html/wrapper.go | 9 ++++++++- render/html/wrapper_test.go | 16 ++++++++++++++++ 4 files changed, 28 insertions(+), 26 deletions(-) diff --git a/render/html/html.go b/render/html/html.go index fa598c3..ff8db14 100644 --- a/render/html/html.go +++ b/render/html/html.go @@ -77,18 +77,12 @@ func (r *Renderer) renderCode(w io.Writer, cell schema.Cell) error { return nil } - div.Open(w, attributes{"class": {"cm-editor", "cm-s-jupyter"}}, true) - div.Open(w, attributes{"class": {"highlight", "hl-ipython3"}}, true) - io.WriteString(w, "
")
 	w.Write(code.Text())
 	io.WriteString(w, "
") - div.Close(w) - div.Close(w) - return nil } diff --git a/render/html/html_test.go b/render/html/html_test.go index 86ed678..38b3e2f 100644 --- a/render/html/html_test.go +++ b/render/html/html_test.go @@ -76,29 +76,14 @@ func TestRenderer(t *testing.T) { Lang: "python", }, want: &node{ - tag: "div", - attr: map[string][]string{ - "class": {"cm-editor", "cm-s-jupyter"}, - }, + tag: "pre", children: []*node{ { - tag: "div", + tag: "code", attr: map[string][]string{ - "class": {"highlight"}, - }, - children: []*node{ - { - tag: "pre", - children: []*node{ - { - tag: "code", - attr: map[string][]string{ - "class": {"language-python"}, - }, - content: "print('Hi, mom!')", - }, - }}, + "class": {"language-python"}, }, + content: "print('Hi, mom!')", }, }, }, diff --git a/render/html/wrapper.go b/render/html/wrapper.go index 42987e2..9a90f31 100644 --- a/render/html/wrapper.go +++ b/render/html/wrapper.go @@ -79,6 +79,9 @@ func (wr *Wrapper) WrapInput(w io.Writer, cell schema.Cell, render render.Render }, "data-type": {"inline"}, }, true) + + div.Open(w, attributes{"class": {"cm-editor", "cm-s-jupyter"}}, true) + div.Open(w, attributes{"class": {"highlight", "hl-ipython3"}}, true) } else if isMd { div.Open(w, attributes{ "class": { @@ -93,7 +96,11 @@ func (wr *Wrapper) WrapInput(w io.Writer, cell schema.Cell, render render.Render // Cell itself _ = render(w, cell) - if isCode || isMd { + if isCode { + div.Close(w) + div.Close(w) + div.Close(w) + } else if isMd { div.Close(w) } diff --git a/render/html/wrapper_test.go b/render/html/wrapper_test.go index db95d7d..83c2180 100644 --- a/render/html/wrapper_test.go +++ b/render/html/wrapper_test.go @@ -201,6 +201,22 @@ func TestWrapper_WrapInput(t *testing.T) { }, "data-type": {"inline"}, }, + children: []*node{ + { + tag: "div", + attr: map[string][]string{ + "class": {"cm-editor", "cm-s-jupyter"}, + }, + children: []*node{ + { + tag: "div", + attr: map[string][]string{ + "class": {"highlight"}, + }, + }, + }, + }, + }, }, }, }, From 240c05350a90d8ca36f5627dd8150ae119cfb2ef Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Mon, 26 Feb 2024 19:23:23 +0100 Subject: [PATCH 2/5] test: fix error printing when comparing HTML structure Drop superficial nodes with '\n' content --> their only purpose is making the generated HTML more readable. They should be disregarded during tests. --- render/html/wrapper_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/render/html/wrapper_test.go b/render/html/wrapper_test.go index 83c2180..fe7c9de 100644 --- a/render/html/wrapper_test.go +++ b/render/html/wrapper_test.go @@ -702,7 +702,7 @@ func findFirst(n *stdhtml.Node, target string) *stdhtml.Node { // Add modifications are done on the copy, so the original node is not modified. func dropChildElements(n *stdhtml.Node) *stdhtml.Node { cp := *n - if cp.FirstChild != nil && cp.FirstChild.Type != stdhtml.TextNode { + if fc := cp.FirstChild; fc != nil && (fc.Type != stdhtml.TextNode || fc.Data == "\n") { cp.FirstChild = nil } cp.NextSibling = nil From b51732d28413afccd5faf9bd582e04a5cb6d20cb Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Tue, 27 Feb 2024 18:06:38 +0100 Subject: [PATCH 3/5] feat: wrap the whole notebook in jp-Notebook div CSS, if any, is now written during the call to WrapAll. This only happens once per notebook, so wrapping CSSWriter into WriterOnce is redundant. Removed WriterOnce, it's not used anywhere else. Added a test case for SVG images in html.Renderer. --- pkg/test/render.go | 1 + render/html/html.go | 4 ++-- render/html/html_test.go | 23 ++++++++--------------- render/html/wrapper.go | 30 ++++++++++-------------------- render/html/wrapper_test.go | 16 ++++++++++++++++ render/render.go | 11 ++++++++--- 6 files changed, 45 insertions(+), 40 deletions(-) diff --git a/pkg/test/render.go b/pkg/test/render.go index 78b65ba..860af44 100644 --- a/pkg/test/render.go +++ b/pkg/test/render.go @@ -17,6 +17,7 @@ type fakeWrapper struct{} var _ render.CellWrapper = (*fakeWrapper)(nil) func (*fakeWrapper) RegisterFuncs(render.RenderCellFuncRegistry) {} +func (*fakeWrapper) WrapAll(io.Writer, func(io.Writer) error) error { return nil } func (*fakeWrapper) Wrap(w io.Writer, c schema.Cell, r render.RenderCellFunc) error { return r(w, c) } func (*fakeWrapper) WrapInput(w io.Writer, c schema.Cell, r render.RenderCellFunc) error { return r(w, c) diff --git a/render/html/html.go b/render/html/html.go index ff8db14..699c97a 100644 --- a/render/html/html.go +++ b/render/html/html.go @@ -15,10 +15,10 @@ type Config struct { type Option func(*Config) -// WithCSSWriter +// WithCSSWriter registers a writer for CSS stylesheet. func WithCSSWriter(w io.Writer) Option { return func(c *Config) { - c.CSSWriter = &WriterOnce{w: w} + c.CSSWriter = w } } diff --git a/render/html/html_test.go b/render/html/html_test.go index 38b3e2f..0b1e06f 100644 --- a/render/html/html_test.go +++ b/render/html/html_test.go @@ -66,6 +66,13 @@ func TestRenderer(t *testing.T) { "src": {"data:image/jpeg;base64, base64-encoded-image"}, }}, }, + { + name: "image/svg+xml", + cell: test.DisplayData("svg-image", "image/svg+xml"), + want: &node{tag: "img", attr: map[string][]string{ + "src": {"data:image/svg+xml;base64, svg-image"}, + }}, + }, { name: "code cell", cell: &test.CodeCell{ @@ -108,20 +115,6 @@ func TestRenderer(t *testing.T) { } func TestRenderer_CSSWriter(t *testing.T) { - t.Run("WithCSSWriter wraps in WriterOnce", func(t *testing.T) { - // Arrange - var cfg html.Config - opt := html.WithCSSWriter(io.Discard) - - // Act - opt(&cfg) - - // Assert - if _, ok := cfg.CSSWriter.(*html.WriterOnce); !ok { - t.Errorf("expected *html.WriterOnce, got %T", cfg.CSSWriter) - } - }) - t.Run("captures correct css", func(t *testing.T) { // Arrange var css bytes.Buffer @@ -134,7 +127,7 @@ func TestRenderer_CSSWriter(t *testing.T) { } // Act - err = r.Wrap(io.Discard, test.Markdown(""), noopRender) + err = r.WrapAll(io.Discard, func(w io.Writer) error { return nil }) require.NoError(t, err) // Assert diff --git a/render/html/wrapper.go b/render/html/wrapper.go index 9a90f31..537e5b8 100644 --- a/render/html/wrapper.go +++ b/render/html/wrapper.go @@ -5,7 +5,6 @@ import ( "io" "sort" "strings" - "sync" "github.com/bevzzz/nb/render" "github.com/bevzzz/nb/schema" @@ -15,9 +14,8 @@ import ( /* TODO: - - make class prefixes configurable (probably on the html.Renderer level). + - make class prefixes configurable (probably on the html.Config level). - refactor to use tagger - - add WrapAll / WrapNotebook that would add a