Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Oct 20, 2025

Implements an optional :border: directive for images to improve contrast with the page background, especially for screenshots with white or light backgrounds from the product UI.

Changes Made

  • Added :border: directive option to the image block that applies a 1px border with color-grey-20 (#e5e9f0)
  • Backend implementation: Added Border property to ImageBlock.cs, ImageViewModel.cs, and updated the HTML renderer
  • Frontend styling: Added .border CSS class in images.css with 1px border using color-grey-20
  • Template updates: Modified ImageView.cshtml to dynamically apply CSS classes
  • Documentation: Added new "Borders" section in images.md with usage examples
  • Tests: Created ImageBlockWithBorderTests to verify the border functionality

Usage

:::{image} /syntax/images/observability.png
:alt: Elasticsearch with border
:width: 400px
:border:
:::

Key Features

  • Optional border via :border: directive
  • 1px wide border with color-grey-20 for subtle contrast
  • Only applies to image directive (inline images excluded as specified)
  • Can be combined with other options like :screenshot: and :width:
  • Fully tested and documented

Fixes #2064

Original prompt

This section details on the original issue you should resolve

<issue_title>Image styling: borders</issue_title>
<issue_description>In a recent conversation about centering images, we also slightly touched on the contrast issue with screenshots originating from the product having the same background color as the docs page background. The fact that our images don't have borders impairs a bit the readability and scannability of our docs pages, especially ones where we need more screenshots than usual, such as visualization-related features.

This is a bigger issue now than in the past because of recent UI theme changes made to the product, where the white-ish background is a lot more present than in previous versions where the product background was grey more often, thus creating more natural contrast and distinctions on docs pages with screenshots.

Example:

Image

Some options:

  • auto-add a solid border to images
  • make it optional using a new styling parameter (we could debate whether it's on or off by default)

Also a note that this shouldn't apply to inline images such as icons.

cc @theletterf </issue_description>

<agent_instructions>Also update the images.md documentation, so we can see examples. Go for an approach where the border is adding optionally through a setting in the image directive, :border:. The border should be 1px wide, color-grey-20. Inline images should not have a border in any case. Only images served through the image directive.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

Fixes #2064


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot finished work on behalf of theletterf October 20, 2025 18:00
@Copilot Copilot AI requested a review from theletterf October 20, 2025 18:00
@theletterf
Copy link
Contributor

@copilot You're getting in the weeds with npm. :) You just need to run this to rebuild the site's CSS and static assets:

npm run build

From the /src/Elastic.Documentation.Site folder.

@Copilot
Copy link
Contributor Author

Copilot AI commented Oct 20, 2025

@copilot You're getting in the weeds with npm. :) You just need to run this to rebuild the site's CSS and static assets:

...

Thanks for the guidance! I've successfully implemented the border feature using npm run build.

The implementation is complete in commit abeb409:

  • Added :border: option to image directive
  • Border is 1px wide with color-grey-20 (#e5e9f0)
  • Works only for images via the image directive (not inline images)
  • Can be combined with other options like :screenshot:
  • Includes tests and documentation examples

The CSS was successfully built and the feature is working as expected. You can see the border class in the generated HTML:

<img ... class="border" />

With CSS:

.markdown-content .border {
    border-style: var(--tw-border-style);
    border-width: 1px;
    border-color: var(--color-grey-20);
}

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • elastic-docs-link-index.s3.us-east-2.amazonaws.com
    • Triggering command: /home/REDACTED/work/docs-builder/docs-builder/.artifacts/bin/docs-builder/debug/docs-builder (dns block)
    • Triggering command: /home/REDACTED/work/docs-builder/docs-builder/.artifacts/bin/docs-builder/debug/docs-builder serve (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@Copilot Copilot AI changed the title [WIP] Add borders to images for better contrast Add optional border styling to images for better contrast Oct 20, 2025
Copilot finished work on behalf of theletterf October 20, 2025 18:20
@github-actions
Copy link

github-actions bot commented Oct 20, 2025

🔍 Preview links for changed docs

@theletterf theletterf marked this pull request as ready for review October 20, 2025 18:24
@theletterf theletterf requested review from a team as code owners October 20, 2025 18:24
@theletterf theletterf requested a review from Mpdreamz October 20, 2025 18:24
@theletterf
Copy link
Contributor

@florent-leborgne
Copy link
Contributor

The rendering LGTM and we can tweak the border styling later if needed, but I wouldn't say this suffices to close the issue as this is currently only taking into account images added using the corresponding directive.

For images using the regular markdown syntax, we're able to pass parameters such as alt text, size... like this:
![title](path "parameter parameter parameter"). I'd like to get input from the team about having one of these parameters to control borders as well, or to have some kind of discussion in the issue about which should be the default behavior.

@colleenmcginnis
Copy link
Contributor

colleenmcginnis commented Oct 21, 2025

Is it not sufficient to use the screenshot attribute instead? https://elastic.github.io/docs-builder/syntax/images/#screenshots If not, when should you use border vs screenshot?

@theletterf
Copy link
Contributor

@colleenmcginnis I suggested that. I think @florent-leborgne would probably go beyond that and use it for all images. So, currently, the PR probably doesn't make anyone happy. :D

I don't think it's the norm to use the :::image component when we add images when we have the regular md syntax that works just fine. An option that also works there would be fine, though i'd make it true by default with the option to remove it

@colleenmcginnis
Copy link
Contributor

colleenmcginnis commented Oct 21, 2025

I think @florent-leborgne would probably go beyond that and use it for all images. So, currently, the PR probably doesn't make anyone happy. :D

If that's the case, I agree that we should consider just adding :screenshot: by default and allowing users to set :screenshot: false in the event that we don't want the border / drop shadow.

IMO If we can't articulate when to use border vs screenshot, that's a sign to me that we're not approaching the problem in the right way. If we're adding border because we don't like the styling of screenshot, we should just update the styling of screenshot rather than introducing another option.

@theletterf
Copy link
Contributor

If we're adding border because we don't like the styling of screenshot

Since Experience team is the one that produces more screenshots, I'll defer to @florent-leborgne here.

we should just update the styling of screenshot rather than introducing another option.

I like this approach best, and then decide whether we want the new screenshot border on by default or not.

@theletterf theletterf marked this pull request as draft October 21, 2025 15:18
@florent-leborgne
Copy link
Contributor

sorry this sounded like a quick improvement but is turning into a longer discussion :D It happens!

we should just update the styling of screenshot rather than introducing another option.

I don't know if I like this option. Because I think all non-inline visuals (diagrams, videos, screenshots, gifs) should get the same general styling (border, position, etc.) by default unless for a specific one we need to work around that default for some reason and opt out.
This is for site consistency and general readability reasons more than to add another opt-in option that will lead to even more inconsistencies :D

For that reason, I think we cannot implement this partially just on image directives (though yes, ok, that is a good start), and that the "screenshot" parameter doesn't really mean something obvious (why would we style a screenshot differently than another visual?). Maybe it's more about renaming it then. But again this only partially address the original intent of the issue.

If we can't agree on this now, this is fine too! This discussion can wait until we have a designer to weigh in and provide their sentiment on the matter. It's not urgent at all.

@colleenmcginnis
Copy link
Contributor

I agree that we should consider just adding :screenshot: by default and allowing users to set :screenshot: false in the event that we don't want the border / drop shadow.

@florent-leborgne sorry I wasn't more specific with this ☝️ . I was thinking we would make the screenshot styling applicable to all images by default regardless of how they are defined in the source file (though probably limited to block images) and the contributor would need to use the directive only if they wanted to remove the screenshot styling.

Maybe it's more about renaming it then.

Yes. If we don't think screenshot is intuitive, then IMO we should deprecate it in favor of border unless we can articulate when to use a border vs screenshot styling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Image styling: borders

5 participants