From e2deea1d5bad1deffa538d5d3b399c8fde142c9b Mon Sep 17 00:00:00 2001 From: Warya Wayne <126521894+WaryaWayne@users.noreply.github.com> Date: Thu, 2 Jul 2026 04:00:19 -0400 Subject: [PATCH] markdown: Make linked images clickable (#59525) # Objective Make images nested inside markdown links (e.g., README badges) clickable, so they behave like normal markdown links. Previously, linked images rendered correctly but were not interactive in markdown previews: they did not show a pointer cursor, could not be clicked to open their target URL, and did not expose link actions through the context menu. ## Solution Changed the `MarkdownElement` to detect when an `` tag is inside a `` tag during rendering. When a linked image is rendered: - The wrapper `div` gets a `cursor_pointer` style and an `on_click` handler that opens the link URL (or delegates to the `on_url_click` callback if set). - The wrapper also captures right-click events to populate the context menu with "Copy Link" via `capture_for_context_menu`. - The `on_url_click` closure type was changed from `Box` to `Rc` to allow cloning across multiple handlers. - When a linked image fails to load, the fallback element's `on_click` now calls `cx.stop_propagation()` to prevent the event from bubbling to the parent link wrapper and opening the URL twice. The feature is gated behind `!self.style.prevent_mouse_interaction` so it respects existing interaction-prevention settings. ## Testing - `cargo test -p markdown`: 95 tests passed. - Manually verified in markdown preview that badge/image links open correctly and right-click shows "Copy Link". - Verified existing markdown tests continue to pass. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [ ] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable --- Release Notes: - Fixed images inside markdown links not being clickable. https://github.com/user-attachments/assets/df2a8d8f-b002-47f9-b139-815ee713700b --------- Co-authored-by: Smit Barmase --- Cargo.lock | 1 + crates/markdown/Cargo.toml | 1 + crates/markdown/src/markdown.rs | 172 ++++++++++++++++++++++++++++---- 3 files changed, 157 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2445366c12..8037aa43f9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10665,6 +10665,7 @@ dependencies = [ "gpui", "gpui_platform", "html5ever 0.27.0", + "image", "language", "languages", "linkify", diff --git a/crates/markdown/Cargo.toml b/crates/markdown/Cargo.toml index 7443803346..3db8d84b73 100644 --- a/crates/markdown/Cargo.toml +++ b/crates/markdown/Cargo.toml @@ -46,6 +46,7 @@ env_logger.workspace = true fs = {workspace = true, features = ["test-support"]} gpui = { workspace = true, features = ["test-support"] } gpui_platform = { workspace = true, features = ["wayland", "x11"] } +image.workspace = true language = { workspace = true, features = ["test-support"] } languages = { workspace = true, features = ["load-grammars"] } node_runtime.workspace = true diff --git a/crates/markdown/src/markdown.rs b/crates/markdown/src/markdown.rs index 024314ebdd..69f5aa3208 100644 --- a/crates/markdown/src/markdown.rs +++ b/crates/markdown/src/markdown.rs @@ -1221,7 +1221,7 @@ pub struct MarkdownElement { markdown: Entity, style: MarkdownStyle, code_block_renderer: CodeBlockRenderer, - on_url_click: Option>, + on_url_click: Option>, code_span_link: Option, on_source_click: Option, on_checkbox_toggle: Option, @@ -1280,7 +1280,7 @@ impl MarkdownElement { mut self, handler: impl Fn(SharedString, &mut Window, &mut App) + 'static, ) -> Self { - self.on_url_click = Some(Box::new(handler)); + self.on_url_click = Some(Rc::new(handler)); self } @@ -1378,18 +1378,58 @@ impl MarkdownElement { width: Option, height: Option, ) { - let image_element = div().min_w_0().child( - img(source) - .id(("markdown-image", range.start)) - .min_w_0() - .max_w_full() - .rounded_md() - .mr_1() - .mb_1() - .when_some(height, |this, height| this.h(height)) - .when_some(width, |this, width| this.w(width)) - .with_fallback(move || image_fallback_element(dest_url.clone(), alt_text.clone())), - ); + let enclosing_link_url = (builder.link_depth > 0) + .then(|| builder.rendered_links.last()) + .flatten() + .map(|link| link.destination_url.clone()); + let fallback_opens_image_url = enclosing_link_url.is_none(); + + let image_element = { + let wrapper = div().id(("markdown-image-link", range.start)).min_w_0(); + let wrapper = if !self.style.prevent_mouse_interaction + && let Some(url) = enclosing_link_url + { + let click_url = url.clone(); + let markdown = self.markdown.clone(); + let url_click = self.on_url_click.clone(); + wrapper + .cursor_pointer() + .on_click(move |_, window, cx| { + if let Some(ref on_url_click) = url_click { + on_url_click(click_url.clone(), window, cx); + } else { + cx.open_url(&click_url); + } + }) + .capture_any_mouse_down(move |event, _window, cx| { + if event.button == MouseButton::Right { + markdown.update(cx, |md, _| { + md.capture_for_context_menu(Some(url.clone()), None) + }); + } + }) + } else { + wrapper + }; + wrapper.child( + img(source) + .id(("markdown-image", range.start)) + .min_w_0() + .max_w_full() + .rounded_md() + .mr_1() + .mb_1() + .when_some(height, |this, height| this.h(height)) + .when_some(width, |this, width| this.w(width)) + .with_fallback(move || { + image_fallback_element( + dest_url.clone(), + alt_text.clone(), + fallback_opens_image_url, + ) + }), + ) + }; builder.push_image_child(image_element); } @@ -2795,7 +2835,11 @@ fn collect_image_alt_text( } } -fn image_fallback_element(dest_url: SharedString, alt_text: Option) -> AnyElement { +fn image_fallback_element( + dest_url: SharedString, + alt_text: Option, + open_image_url_on_click: bool, +) -> AnyElement { let link_label = alt_text .filter(|alt| !alt.is_empty()) .unwrap_or_else(|| dest_url.clone()); @@ -2804,13 +2848,15 @@ fn image_fallback_element(dest_url: SharedString, alt_text: Option div() .id("image-fallback") - .cursor_pointer() .min_w_0() .child(Label::new(label).color(Color::Warning).underline()) .tooltip(Tooltip::text( "Image failed to load. Open `zed: log` for more details.", )) - .on_click(move |_, _, cx| cx.open_url(&dest_url)) + .when(open_image_url_on_click, |this| { + this.cursor_pointer() + .on_click(move |_, _, cx| cx.open_url(&dest_url)) + }) .into_any_element() } @@ -5289,6 +5335,98 @@ mod tests { }); } + fn failing_image_source() -> ImageSource { + ImageSource::Custom(Arc::new(|_, _| { + Some(Err(gpui::ImageCacheError::Asset( + "failed to load image".into(), + ))) + })) + } + + fn loaded_image_source() -> ImageSource { + let buffer = image::ImageBuffer::from_pixel(16, 16, image::Rgba([0, 0, 0, 255])); + ImageSource::Render(Arc::new(gpui::RenderImage::new(SmallVec::from_elem( + image::Frame::new(buffer), + 1, + )))) + } + + fn open_markdown_image_test_window<'a>( + source: &str, + image_source: ImageSource, + cx: &'a mut TestAppContext, + ) -> &'a mut gpui::VisualTestContext { + struct ImageTestView { + markdown: Entity, + image_source: ImageSource, + } + + impl Render for ImageTestView { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + let image_source = self.image_source.clone(); + div().size_full().child( + MarkdownElement::new(self.markdown.clone(), MarkdownStyle::default()) + .image_resolver(move |_| Some(image_source.clone())), + ) + } + } + + ensure_theme_initialized(cx); + + let source = source.to_string(); + let (_, cx) = cx.add_window_view(|_, cx| ImageTestView { + markdown: cx.new(|cx| Markdown::new(source.into(), None, None, cx)), + image_source, + }); + cx.run_until_parked(); + cx + } + + #[gpui::test] + fn test_clicking_image_fallback_opens_image_url(cx: &mut TestAppContext) { + let cx = open_markdown_image_test_window( + "![alt text](https://example.com/image.png)", + failing_image_source(), + cx, + ); + + cx.simulate_click(point(px(8.), px(8.)), gpui::Modifiers::default()); + assert_eq!( + cx.opened_url(), + Some("https://example.com/image.png".to_string()) + ); + } + + #[gpui::test] + fn test_clicking_image_fallback_inside_link_opens_link_url(cx: &mut TestAppContext) { + let cx = open_markdown_image_test_window( + "[![alt text](https://example.com/image.png)](https://example.com/link)", + failing_image_source(), + cx, + ); + + cx.simulate_click(point(px(8.), px(8.)), gpui::Modifiers::default()); + assert_eq!( + cx.opened_url(), + Some("https://example.com/link".to_string()) + ); + } + + #[gpui::test] + fn test_clicking_loaded_image_inside_link_opens_link_url(cx: &mut TestAppContext) { + let cx = open_markdown_image_test_window( + "[![alt text](https://example.com/image.png)](https://example.com/link)", + loaded_image_source(), + cx, + ); + + cx.simulate_click(point(px(8.), px(8.)), gpui::Modifiers::default()); + assert_eq!( + cx.opened_url(), + Some("https://example.com/link".to_string()) + ); + } + #[track_caller] fn assert_mappings(rendered: &RenderedText, expected: Vec>) { assert_eq!(rendered.lines.len(), expected.len(), "line count mismatch");