Skip to content

feat: Add docs for DownloadHandler #4309

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

Merged
merged 18 commits into from
Jun 18, 2025
Merged

feat: Add docs for DownloadHandler #4309

merged 18 commits into from
Jun 18, 2025

Conversation

mshabarov
Copy link
Contributor

Adds a documentation for new API for handling downloads in Vaadin 24.8.

Fixes #4303

Copy link

github-actions bot commented May 14, 2025

AI Language Review

The provided files are new, so I am reviewing only the ".new" versions for clarity, accuracy, and completeness.

  1. Dynamic Content.adoc:

    • Under the "Using Custom Servlet and Request Parameters" section, the phrase "You can create a custom servlet which handles 'image' as a relative URL" should be clarified. Consider rephrasing to specify that this involves configuring a new servlet to respond to requests at the "/image" path.
    • Consider specifying that the @WebServlet annotation requires import from javax.servlet.annotation so readers know it's not provided out of the box in some environments.
    • The explanation of SVG XML could be expanded slightly to ensure readers understand they might need to adjust or expand the SVG content as needed for different use cases.
    • In the "Using StreamResource" section, consider more explicitly stating that StreamResource allows integration without managing servlet configurations.
  2. Downloads.adoc:

    • When describing "Render Or Download Static Resource," clarify that a preceding slash in the resource name specifies an absolute path, affecting where it will look for the resource.
    • In "Download A File From File System," add that file path specification should be secure and caution against exposing sensitive directories.
    • Under "Download Progress Listeners," it could be beneficial to explicitly state the possible need for thread safety when using certain callbacks, even though Vaadin manages UI.access().
    • For the "Disabled Update Mode" section, the explanation around specifying that it overrides the default behavior of downloads could be elaborated to emphasize implications on security and user experience.
    • In "URL Postfix," the explanation can be expanded to clarify what part of the URL structure it affects and why it might be beneficial or necessary to use in dynamic URL generation scenarios.

The documentation is generally comprehensive and well-structured. Just make sure to clarify the contexts and implications of technical configurations for users who might be new to servlet and Vaadin environments.

@mshabarov mshabarov requested review from Copilot and caalador May 14, 2025 13:45
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds comprehensive documentation for the new DownloadHandler API in Vaadin 24.8, including multiple usage scenarios and API customization examples.

  • Removed the legacy dynamic content documentation.
  • Added detailed documentation for DownloadHandler covering classpath resource downloads, file system downloads, dynamic InputStream downloads, progress tracking, and custom download handlers.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
articles/flow/advanced/dynamic-content.adoc Legacy documentation removed.
articles/flow/advanced/downloads.adoc New documentation added for the DownloadHandler API.
Comments suppressed due to low confidence (1)

articles/flow/advanced/downloads.adoc:241

  • [nitpick] The class name 'LinkWithM5Validation' is ambiguous; consider renaming it to something more descriptive, such as 'LinkWithChecksumValidation'.
private static class LinkWithM5Validation extends Anchor {

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@mshabarov mshabarov marked this pull request as draft May 23, 2025 07:37
@mshabarov
Copy link
Contributor Author

Converted to draft to not being merged before RC/GA

@mshabarov
Copy link
Contributor Author

I've added the recently introduced AttachmentType and placed the DownloadEvent and lambda example in the first place in the list, also reordered the helpers a bit.

@mshabarov mshabarov requested a review from caalador June 17, 2025 12:06
@mshabarov mshabarov marked this pull request as ready for review June 18, 2025 06:19
@mshabarov mshabarov merged commit 7e8ccd5 into latest Jun 18, 2025
4 of 5 checks passed
@mshabarov mshabarov deleted the download-handler branch June 18, 2025 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation for the new Download API
3 participants