Skip to content

[Top Navigation] Close actions popover on click#203830

Merged
kowalczyk-krzysztof merged 2 commits intoelastic:mainfrom
kowalczyk-krzysztof:fix/topnav-mobile-close-popover-onclick
Dec 16, 2024
Merged

[Top Navigation] Close actions popover on click#203830
kowalczyk-krzysztof merged 2 commits intoelastic:mainfrom
kowalczyk-krzysztof:fix/topnav-mobile-close-popover-onclick

Conversation

@kowalczyk-krzysztof
Copy link
Copy Markdown
Member

@kowalczyk-krzysztof kowalczyk-krzysztof commented Dec 11, 2024

Summary

This PR makes it so the actions popover closes after clicking on an item.

Closes: #201358

@kowalczyk-krzysztof kowalczyk-krzysztof added bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// backport:prev-minor labels Dec 11, 2024
@kowalczyk-krzysztof kowalczyk-krzysztof self-assigned this Dec 11, 2024
@kowalczyk-krzysztof kowalczyk-krzysztof requested a review from a team as a code owner December 11, 2024 15:53
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@eokoneyo
Copy link
Copy Markdown
Contributor

Hi @kowalczyk-krzysztof I took a look at this, whilst this PR fixes the particularly issue mentioned, I was able to reproduce the same in other parts of the app, see the video linked below;

Screen.Recording.2024-12-13.at.09.52.54.mov

Could we apply this fix in such a way that it's global?

@kowalczyk-krzysztof
Copy link
Copy Markdown
Member Author

@eokoneyo Thanks for checking this. I thought the component is reused across top navigation - but I guess not. I'll look into it.

@kowalczyk-krzysztof kowalczyk-krzysztof marked this pull request as draft December 13, 2024 11:15
@kowalczyk-krzysztof
Copy link
Copy Markdown
Member Author

kowalczyk-krzysztof commented Dec 16, 2024

So it seems that for non-solution specific code this component is used:
https://github.com/elastic/kibana/blob/main/src/plugins/navigation/public/top_nav_menu/top_nav_menu_items.tsx - it's also used for some Management pages.

However, each solutions seems to have its own implementation of this component:

I think this is a great candidate for a shared component, I will open a follow up issue for this.

EDIT: Follow up issue: #204347

@kowalczyk-krzysztof kowalczyk-krzysztof marked this pull request as ready for review December 16, 2024 09:51
Copy link
Copy Markdown
Contributor

@eokoneyo eokoneyo left a comment

Choose a reason for hiding this comment

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

Changes LGTM, tested locally. We decided that the previous concern pertaining to other similar instances of the same issue will be resolved in a different issue.

@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #15 / MultipleAgentPolicySummaryLine it should render the first policy name and the badge when there are multiple policies

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
navigation 18.3KB 18.3KB +55.0B

History

cc @kowalczyk-krzysztof

@kowalczyk-krzysztof kowalczyk-krzysztof merged commit 3d79542 into elastic:main Dec 16, 2024
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12351877828

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 16, 2024
## Summary

This PR makes it so the actions popover closes after clicking on an
item.

Closes: elastic#201358
(cherry picked from commit 3d79542)
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Dec 16, 2024
)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Top Navigation] Close actions popover on click
(#203830)](#203830)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Krzysztof
Kowalczyk","email":"krzysztof.kowalczyk@elastic.co"},"sourceCommit":{"committedDate":"2024-12-16T11:39:48Z","message":"[Top
Navigation] Close actions popover on click (#203830)\n\n##
Summary\r\n\r\nThis PR makes it so the actions popover closes after
clicking on an\r\nitem.\r\n\r\nCloses:
#201358","sha":"3d79542b204ad58a5441d820ba7b2d0d6724f305","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","v9.0.0","Team:SharedUX","backport:prev-minor"],"title":"[Top
Navigation] Close actions popover on
click","number":203830,"url":"https://github.com/elastic/kibana/pull/203830","mergeCommit":{"message":"[Top
Navigation] Close actions popover on click (#203830)\n\n##
Summary\r\n\r\nThis PR makes it so the actions popover closes after
clicking on an\r\nitem.\r\n\r\nCloses:
#201358","sha":"3d79542b204ad58a5441d820ba7b2d0d6724f305"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/203830","number":203830,"mergeCommit":{"message":"[Top
Navigation] Close actions popover on click (#203830)\n\n##
Summary\r\n\r\nThis PR makes it so the actions popover closes after
clicking on an\r\nitem.\r\n\r\nCloses:
#201358","sha":"3d79542b204ad58a5441d820ba7b2d0d6724f305"}}]}]
BACKPORT-->

Co-authored-by: Krzysztof Kowalczyk <krzysztof.kowalczyk@elastic.co>
JoseLuisGJ pushed a commit to JoseLuisGJ/kibana that referenced this pull request Dec 19, 2024
## Summary

This PR makes it so the actions popover closes after clicking on an
item.

Closes: elastic#201358
@kowalczyk-krzysztof kowalczyk-krzysztof deleted the fix/topnav-mobile-close-popover-onclick branch August 31, 2025 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// v8.18.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Top Navigation] Actions popover does not hide after selecting an option, requiring manual intervention to hide it.

4 participants