Making an AnyElement scrollable

[kierangilliam]

I'm working on a markdown preview action.

Preview:

zed.-.markdown.preview.mov

I'm reusing the workspace's rich_text crate:

impl Render for MarkdownPreviewView {
    fn render(&mut self, cx: &mut ViewContext<Self>) -> impl IntoElement {
        let container = div()
            .key_context("MarkdownPreview")
            .p_4()
            .gap_4();

        let editor = self.active_editor.read(cx);
        let contents = editor.buffer().read(cx).snapshot(cx).text();

        let mentions = vec![];
        let text = rich_text::render_markdown(contents, &mentions, &self.languages, None);

        let md_container = div()
            .id("markdown_preview_container") # A
            .gap_2()
            .overflow_y_scroll() # A
            .child(text.element("body".into(), cx)); # B

        container.child(md_container)
    }
}

I'm unable to make the markdown preview scrollable.

A) I noticed that I'm only able to apply .overflow_y_scroll() if I assign a div an id. I'm struggling to understand why this is. I understand id returns a Stateful instance of the div which implements InteractiveElement, but I don't understand why that's not the default.

B) Applying overflow on the parent div does not make .child(text.element("body".into(), cx)); scrollable as expected. What's the correct path forward? To cast text.element("body".into(), cx) back into an InteractiveText and then to implement InteractiveElement for InteractiveText?

[mikayla-maki]

In GPUI, the element tree you construct in ‘render’ is only used for a single frame. But some elements need some minor state tracking in order to provide the services they do, such as the current scroll offset. In order to carry this state across frames, we use the element id to lookup the element’s state. The ID is optional because you only really need to do this for certain kinds of interactivity, for common cases, this is useless clutter.

As for the scroll issue, you might need to add a ‘size_full()’ to your div so it knows what bounds it’s in. if that doesn’t fix it, let me know and I’ll be at my desk to take a closer look later :)

[SomeoneToIgnore]

To add to the last part of the previous answer (sorry, was typing that but too long): there has to be be a max_h size set in order to enable the scrolling: otherwise, layouting system (taffy) does consider that the element is "infinite" and renders the scrolling as noop.

Albeit your PR is on early stages, a few notes I'd love to add there:

• it's great, thank you for working on this
• please do make it open in a split pane instead of a new one (similar to what syntax tree browser does)
• please update the license field in the crate — all others have a different one
• tree-sitter-markdown dependency in the new crate seems redundant — if it's not, after all, it would be great to have a single version used across the entire workspace: with the version defined in the root Cargo.toml and the rest just using the smallvec.workspace = true notation

[kierangilliam]

Thank you, I really appreciate this feedback. Will do.

[kierangilliam]

@SomeoneToIgnore again, thanks for the feedback. I applied your suggestions to my new PR #6958. Depending on which direction you all want to go in, I can also apply these suggestions to the original PR.

there has to be be a max_h size set in order to enable the scrolling

This was the issue, thanks! I noticed that max_h_full doesn't work. What's the recommended way to occupy the entire space in a pane? This is what I'm doing now, but I know that it's incorrect as pane height != viewport height:

impl Render for MarkdownPreviewView {
    fn render(&mut self, cx: &mut ViewContext<Self>) -> impl IntoElement {

        let viewport_height = cx.viewport_size().height;

        let mut container = v_flex()
            .items_start()
            .justify_start()
            .key_context("MarkdownPreview")
            .track_focus(&self.focus_handle)
            .max_h(viewport_height)

please do make it open in a split pane instead of a new one (similar to what syntax tree browser does)

Thanks for pointing me to this example! Anchoring suggestions to existing code is useful as I'm 2 days old to this codebase :). Done.

please update the license field in the crate — all others have a different one

I'm not sure which license you are referring to. When I started, I copy/pasted the feedback crate. I looked through a few different crates that create actions. They use GPL for the most part. This is what markdown_preview also uses. Which license do you want me to use?

tree-sitter-markdown dependency in the new crate seems redundant — if it's not, after all, it would be great to have a single version used across the entire workspace: with the version defined in the root Cargo.toml and the rest just using the smallvec.workspace = true notation

Thanks for the callout. I did not need tree-sitter-markdown but I went ahead and consolidated all of the uses of pulldown-cmark.

[SomeoneToIgnore]

Thank you for the follow-up, I've just checked the video on the new PR and it looks so nice already!
I'll continue in the PR itself, seems that your question here is solved 🙂