Skip to content
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

fix(mrc): fix <ActionMenu /> #14921

Open
wants to merge 1 commit into
base: release/mrc-v2-update
Choose a base branch
from
Open

Conversation

rjamet-ovh
Copy link
Contributor

Question Answer
Branch? develop
Bug fix? yes
New feature? no
Breaking change? no
Tickets Fix #14914
License BSD 3-Clause
  • Try to keep pull requests small so they can be easily reviewed.
  • Commits are signed-off
  • Only FR translations have been updated
  • Branch is up-to-date with target branch
  • Lint has passed locally
  • Standalone app was ran and tested locally
  • Ticket reference is mentioned in linked commits (internal only)
  • Breaking change is mentioned in relevant commits

Description

  • add download file use-case
  • fix menu item style

@rjamet-ovh rjamet-ovh requested review from a team as code owners January 15, 2025 09:11
@github-actions github-actions bot added the bug Something isn't working label Jan 15, 2025
@rjamet-ovh rjamet-ovh linked an issue Jan 15, 2025 that may be closed by this pull request
3 tasks
chipp972
chipp972 previously approved these changes Jan 15, 2025
tibs245
tibs245 previously approved these changes Jan 15, 2025
tristanwagner
tristanwagner previously approved these changes Jan 17, 2025
Copy link
Contributor

@anooparveti anooparveti left a comment

Choose a reason for hiding this comment

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

so the Menu component does have actions only, so Buttons are the only items accepted there (no links). If in your use case you have to navigate, please use the trigger function on the Button to do so

Following the suggestion of Francois, please implement this way.

@@ -21,6 +23,9 @@ export interface ActionMenuItem {
iamActions?: string[];
urn?: string;
className?: string;
isDisabled?: boolean;
isLoading?: boolean;
color?: ODS_BUTTON_COLOR;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
export interface ActionMenuItem {
id: number;
rel?: string;
href?: string;
download?: string;
target?: string;
onClick?: () => void;
label: string;
className?: string;
} & React.ComponentProps<typeof OdsButton>

Copy link
Contributor

@tibs245 tibs245 left a comment

Choose a reason for hiding this comment

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

Just suggestion to improve generic usage of this component

@rjamet-ovh rjamet-ovh changed the base branch from develop to release/mrc-v2-update January 20, 2025 09:52
- add download file use-case
- fix menu item style
- fix trigger btn clickable zone

ref: 14914

Signed-off-by: Romain Jamet <romain.jamet.ext@corp.ovh.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[manager-components]: <ActionMenu />
5 participants