JuliaPluto / PlutoUI.jl

https://featured.plutojl.org/basic/plutoui.jl
The Unlicense
299 stars 54 forks source link

Lift `AbstractString` type restriction on `summary` arg of `details` #296

Closed genericallyterrible closed 3 months ago

genericallyterrible commented 4 months ago

Fixes: https://github.com/JuliaPluto/PlutoUI.jl/issues/295

Allows any type to be used as summary and leaves it up to the user to decide what is rational.

Unlike the details argument, summary is not embedded using embed_display. Instead summary is only wrapped by a simple div with appropriate CSS to provide a decent display for html-like objects.

Examples

summary type Output
String (unchanged) image
@md image
@htl image
other (dict) image
github-actions[bot] commented 4 months ago

Try this Pull Request!

Open Julia and type:

  julia> import Pkg
julia> Pkg.activate(temp=true)
julia> Pkg.add(url="https://github.com/genericallyterrible/PlutoUI.jl", rev="non-str-summary")
julia> using PlutoUI

Or run this code in your browser: Run with binder

genericallyterrible commented 4 months ago

Chrome is currently unhappy :(

image

genericallyterrible commented 4 months ago

Parity on my machine.

Browser Example
Chrome image
Firefox image
genericallyterrible commented 4 months ago

Quoting: @rgouveiamendes from https://github.com/JuliaPluto/PlutoUI.jl/pull/290#issuecomment-1960407133

... Is there any particular reason why you prefer display: inline-block; to display: inline-flex;? The difference I see is the position of the triangle at the top or bottom of the block. ...

No particular conviction, inline-block was the only option I saw when looking for examples of elements nested within a <summary> tag and it looked good to me.

Upon comparison, inline-flex is most similar to default <summary> behavior with simple contents, though I don't think any nested div solution could perfectly mimic default behavior.

Style Result
Default <summary>, no nested <div> image
Nested div with display: inline-block; (current) image
Nested div with display: inline-flex; (proposed) image

While doing this comparison I have become partial to inline-flex, but unfortunately it has its own problems! Yet somehow inline-table appears to be a potential panacea to our present predicament.

Style Result
Nested div with display: inline-flex; image
Nested div with display: inline-table; image

I cannot say I fully comprehend the depth of the implications were we to opt for inline-table. inline-block makes simple sense to my grug brain, but I'll gladly change the display if those with bigger brains say unto me, "grug, inline-table is good."

grug
fonsp commented 4 months ago

I'm really happy with your contributions to PlutoUI! Are you interested in other projects? I'm thinking about:

Do you know JavaScript? In that case there are lots of possible projects similar to https://github.com/lukavdplas/PlutoMapPicker.jl . Also check out the recent new feature: https://github.com/fonsp/Pluto.jl/pull/2726

Anyway, big thanks for your work so far and maybe we can find something fun to work on! I'm also happy to chat on zulip or have a video call to discuss!

rgouveiamendes commented 4 months ago

Wow! @fonsp let me see what I can do. I will analyze the links you've mentioned. However, I am quite busy right now... :( Maybe just in summer.

Regarding flex-inline what are exactly the problems you detected @genericallyterrible? I am using it without any problem in Brave (Chrome).

genericallyterrible commented 4 months ago

Regarding flex-inline what are exactly the problems you detected @genericallyterrible?

My screenshots above could've been clearer and I should've explicitly stated what I found rather than making assumptions, apologies. With inline-flex, summaries made with @md are fantastic and appear exactly as I'd expect but summaries made with @htl aren't nearly as pretty :(

The following screenshot showcases functionally equivalent markdown and html producing wildly different results when using display: inline: flex. image

inline-table is not consistent across Chrome and Firefox so there goes that idea 🙃

Nothing an extra wrapper can't fix though, eh? image

fonsp commented 4 months ago

Looks good! Let's merge?

@rgouveiamendes My invitation for future contributions was meant more for @genericallyterrible who made a couple of really well implemented pull requests recently but of course everyone should feel welcome to contribute! Open a draft PR when you start working and let's make something nice!

genericallyterrible commented 4 months ago

I'm ready to send it!