Skip to content

Conversation

TrevorBurnham
Copy link
Contributor

This PR adds the ability to specify download: true for an item in a ButtonDropdown, in conjunction with an href, to get a download link instead of a normal link. This matches the behavior of the download prop on the Button component.

How has this been tested?

I've added unit tests as part of this PR.

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

This PR adds the ability to specify `download: true` for an item in a
`ButtonDropdown`, in conjunction with an `href`, to get a download link
instead of a normal link. This matches the behavior of the `download`
prop on the `Button` component.
@TrevorBurnham TrevorBurnham requested a review from a team as a code owner July 28, 2025 16:48
@TrevorBurnham TrevorBurnham requested review from orangevolon and removed request for a team July 28, 2025 16:48
@gethinwebster gethinwebster changed the title feat: add support for download links in ButtonDropdown feat: Add support for download links in ButtonDropdown Jul 29, 2025
expect(item.getElement()).toHaveTextContent('Button item');
});

it('should not render download attribute when item is disabled', () => {
Copy link
Member

Choose a reason for hiding this comment

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

The link is already not clickable when disabled. Why do we want to remove the download attribute?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly, Claude added this. I agree that it doesn't really matter. The Button component keeps the download attribute when it's disabled.

@@ -165,7 +167,7 @@ export namespace ButtonDropdownProps {
}

export interface CheckboxItem
extends Omit<ButtonDropdownProps.Item, 'href' | 'external' | 'externalIconAriaLabel' | 'itemType'> {
extends Omit<ButtonDropdownProps.Item, 'href' | 'download' | 'external' | 'externalIconAriaLabel' | 'itemType'> {
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if we need a visual clue for download similar to external.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In practice, I think we'd want to use iconName="download" whenever we use the download attribute. But I don't think that should be enforced; someone might want to use a different (custom?) icon to differentiate some downloads.

Having download and iconName as orthogonal options matches the API of the Button component.

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.

2 participants