Skip to content

feat(menu): add MenuItem action support for onPress, href, and to#1528

Merged
Marcosld merged 6 commits intoTelefonica:masterfrom
MurilloLeoni:feat/menu-item-action-props
May 11, 2026
Merged

feat(menu): add MenuItem action support for onPress, href, and to#1528
Marcosld merged 6 commits intoTelefonica:masterfrom
MurilloLeoni:feat/menu-item-action-props

Conversation

@MurilloLeoni
Copy link
Copy Markdown
Contributor

@MurilloLeoni MurilloLeoni commented Apr 8, 2026

Adds an action prop to MenuItem.

Supports:

  • onPress for click actions
  • href for external links
  • to for internal navigation

Jira: https://jira.tid.es/browse/OBVIVO-3412

@yceballost
Copy link
Copy Markdown
Contributor

Hi @MurilloLeoni, is this PR ready to review? Please, include some reviewers like @Marcosld and me

Please, include in PR body the link of the Jira issue

Thanks!! 🤟

Comment thread playroom/snippets.tsx Outdated
Comment thread src/menu.tsx
Comment thread src/menu.tsx Outdated
@MurilloLeoni
Copy link
Copy Markdown
Contributor Author

Hi! Thank you very much for the feedback and for taking the time to review this PR. 🙂

I’ve added the Jira ticket link to the PR description as requested. For context, the ticket was later cancelled since the development was handled directly by our team, but this is the related one:
https://jira.tid.es/browse/OBVIVO-3412

Regarding the implementation, I initially introduced an action prop to support href, to, and onPress in MenuItem. Based on your feedback, I understand that this approach is not aligned with the current API patterns and feature parity across the library.

I’m currently updating the Menu component to follow the suggested approach.

Comment thread src/menu.tsx Outdated
Comment thread src/menu.tsx
@MurilloLeoni MurilloLeoni requested a review from Marcosld April 13, 2026 14:44
Copy link
Copy Markdown
Contributor

@yceballost yceballost left a comment

Choose a reason for hiding this comment

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

nice!

@MurilloLeoni
Copy link
Copy Markdown
Contributor Author

Hi, sorry for the delay on my side. We had some high-priority tasks in our team and needed to focus on those deliveries, so this one ended up being paused for a bit.

About this PR: I saw that the changes were already approved, but the pipeline is still failing. I checked the errors and they seem to be related to pipeline configuration (e.g., missing token in checkout, accessibility audit failing in CI, and Azure upload issue), not directly to the code changes.

Could you please clarify how the merge process works in this case? Should I take any action to fix these pipeline issues, or is this something handled on your side?

Thanks!

@yceballost
Copy link
Copy Markdown
Contributor

Hi, sorry for the delay on my side. We had some high-priority tasks in our team and needed to focus on those deliveries, so this one ended up being paused for a bit.

About this PR: I saw that the changes were already approved, but the pipeline is still failing. I checked the errors and they seem to be related to pipeline configuration (e.g., missing token in checkout, accessibility audit failing in CI, and Azure upload issue), not directly to the code changes.

Could you please clarify how the merge process works in this case? Should I take any action to fix these pipeline issues, or is this something handled on your side?

Thanks!

I think the problems of the actions is not by your fault. I think the PR can be merge when @Marcosld can do that for the next release

@Marcosld Marcosld merged commit 060f926 into Telefonica:master May 11, 2026
12 of 16 checks passed
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.

4 participants