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

Fix Plotly require.js config mechanism #155

Merged
merged 9 commits into from
Jun 19, 2024
53 changes: 48 additions & 5 deletions src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,60 @@
import QuartoNotebookWorker
import PlotlyBase

QuartoNotebookWorker._mimetype_wrapper(p::PlotlyBase.Plot) = PlotlyBasePlot(p)
struct PlotlyBasePlotWithoutRequireJS
plot::PlotlyBase.Plot
end

struct PlotlyRequireJSConfig end

const FIRST_PLOT_DISPLAYED = Ref(false)

function QuartoNotebookWorker.expand(p::PlotlyBase.Plot)

Check warning on line 14 in src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl#L14

Added line #L14 was not covered by tests

plotcell = QuartoNotebookWorker.Cell(PlotlyBasePlotWithoutRequireJS(p))

Check warning on line 16 in src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl#L16

Added line #L16 was not covered by tests

# Quarto expects that the require.js preamble which Plotly needs to function
# comes in its own cell, which will then be hoisted into the HTML page header.
# So we cannot have that preamble concatenated with every plot's HTML content.
# Instead, e keep track whether a Plotly plot is the first per notebook, and in that
# case have it expand into the preamble cell and the plot cell. If it's not the
# first time, we expand only into the plot cell.
cells = if !FIRST_PLOT_DISPLAYED[]
[QuartoNotebookWorker.Cell(PlotlyRequireJSConfig()), plotcell]

Check warning on line 25 in src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl#L24-L25

Added lines #L24 - L25 were not covered by tests
else
[plotcell]

Check warning on line 27 in src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl#L27

Added line #L27 was not covered by tests
end

FIRST_PLOT_DISPLAYED[] = true

Check warning on line 30 in src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl#L30

Added line #L30 was not covered by tests

struct PlotlyBasePlot <: QuartoNotebookWorker.WrapperType
value::PlotlyBase.Plot
return cells

Check warning on line 32 in src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl#L32

Added line #L32 was not covered by tests
end

function Base.show(io::IO, ::MIME"text/html", wrapper::PlotlyBasePlot)
function Base.show(io::IO, ::MIME"text/html", p::PlotlyBasePlotWithoutRequireJS)

Check warning on line 35 in src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl#L35

Added line #L35 was not covered by tests
# We want to embed only the minimum markup needed to render the
# plotlyjs plots, otherwise a full HTML page is generated for every
# plot which does not render correctly in our context.
PlotlyBase.to_html(io, wrapper.value; include_plotlyjs = "require", full_html = false)
# "require-loaded" means that we pass the require.js preamble ourselves.
PlotlyBase.to_html(io, p.plot; include_plotlyjs = "require-loaded", full_html = false)

Check warning on line 40 in src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl#L40

Added line #L40 was not covered by tests
end

Base.show(io::IO, M::MIME, p::PlotlyBasePlotWithoutRequireJS) = show(io, M, p.plot)
Base.show(io::IO, m::MIME"text/plain", p::PlotlyBasePlotWithoutRequireJS) =

Check warning on line 44 in src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl#L43-L44

Added lines #L43 - L44 were not covered by tests
show(io, m, p.plot)
Base.showable(M::MIME, p::PlotlyBasePlotWithoutRequireJS) = showable(M, p.plot)

Check warning on line 46 in src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl#L46

Added line #L46 was not covered by tests

function Base.show(io::IO, ::MIME"text/html", ::PlotlyRequireJSConfig)
print(io, PlotlyBase._requirejs_config())

Check warning on line 49 in src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl#L48-L49

Added lines #L48 - L49 were not covered by tests
end

function reset_first_plot_displayed_flag!()
FIRST_PLOT_DISPLAYED[] = false

Check warning on line 53 in src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl#L52-L53

Added lines #L52 - L53 were not covered by tests
end

function __init__()
if ccall(:jl_generating_output, Cint, ()) == 0
QuartoNotebookWorker.add_package_refresh_hook!(reset_first_plot_displayed_flag!)

Check warning on line 58 in src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyBaseExt.jl#L56-L58

Added lines #L56 - L58 were not covered by tests
end
end

end
18 changes: 1 addition & 17 deletions src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyJSExt.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,6 @@
import QuartoNotebookWorker
import PlotlyJS

QuartoNotebookWorker._mimetype_wrapper(p::PlotlyJS.SyncPlot) = PlotlyJSSyncPlot(p)

struct PlotlyJSSyncPlot <: QuartoNotebookWorker.WrapperType
value::PlotlyJS.SyncPlot
end

function Base.show(io::IO, ::MIME"text/html", wrapper::PlotlyJSSyncPlot)
# We want to embed only the minimum markup needed to render the
# plotlyjs plots, otherwise a full HTML page is generated for every
# plot which does not render correctly in our context.
PlotlyJS.PlotlyBase.to_html(
io,
wrapper.value.plot;
include_plotlyjs = "require",
full_html = false,
)
end
QuartoNotebookWorker.expand(p::PlotlyJS.SyncPlot) = QuartoNotebookWorker.expand(p.plot)

Check warning on line 6 in src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyJSExt.jl

View check run for this annotation

Codecov / codecov/patch

src/QuartoNotebookWorker/ext/QuartoNotebookWorkerPlotlyJSExt.jl#L6

Added line #L6 was not covered by tests

end
9 changes: 8 additions & 1 deletion test/testsets/integrations/PlotlyJS.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,14 @@ if Sys.iswindows()
else
test_example(joinpath(@__DIR__, "../../examples/integrations/PlotlyJS.qmd")) do json
cells = json["cells"]
for nth in (4, 6)
preamble_cell = cells[5]
outputs = preamble_cell["outputs"]
@test length(outputs) == 1
data = outputs[1]["data"]
@test keys(data) == Set(["text/html", "text/plain"])
@test startswith(data["text/html"], "<script type=\"text/javascript\">")
@test occursin("require.undef(\"plotly\")", data["text/html"])
for nth in (6, 9)
cell = cells[nth]
outputs = cell["outputs"]
@test length(outputs) == 1
Expand Down
Loading