-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[ftr/menuToggle] provide helper for enhanced menu toggle handling #81709
[ftr/menuToggle] provide helper for enhanced menu toggle handling #81709
Conversation
💚 Build SucceededMetrics [docs]
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for migrating the logic into a single place. The new service makes a lot of sense.
LGTM
code review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. MenuToggle
class looks useful.
…astic#81709) Co-authored-by: spalger <spalger@users.noreply.github.com> # Conflicts: # test/functional/services/common/test_subjects.ts
…astic#81709) Co-authored-by: spalger <spalger@users.noreply.github.com> # Conflicts: # test/functional/services/common/test_subjects.ts
* master: (87 commits) [Actions] Adding `hasAuth` to Webhook Configuration to avoid confusing UX (elastic#81778) [i18n] add get_kibana_translation_paths tests (elastic#81624) [UX] Fix search term reset from url (elastic#81654) [Lens] Improved range formatter (elastic#80132) [Resolver] `SideEffectContext` changes, remove `@testing-library` uses (elastic#81077) [Time to Visualize] Update Library Text with Call to Action (elastic#81667) [docs] Resolving failed Kibana upgrade migrations (elastic#80999) [ftr/menuToggle] provide helper for enhanced menu toggle handling (elastic#81709) [Task Manager] adds basic observability into Task Manager's runtime operations (elastic#77868) Doc changes for stack management and grouped feature privileges (elastic#80486) Added functional test for alerts list filters by status, alert type and action type. Did a code refactoring and splitting for alerts tests. (elastic#81422) [Security Solution][Endpoint][Admin] Malware Protections Notify User Version (elastic#81415) Revert "[Actions] Adding `hasAuth` to Webhook Configuration to avoid confusing UX (elastic#81390)" [Maps] Use default format when proxying EMS-files (elastic#79760) [Discover] Deangularize context.html (elastic#81442) Use the displayName property in default editor (elastic#73311) adds bug label to Bug report for Security Solution template (elastic#81643) [ML] Transforms: Remove index field limitation for custom query. (elastic#81467) [Actions] Adding `hasAuth` to Webhook Configuration to avoid confusing UX (elastic#81390) [Task Manager] Mark task as failed if maxAttempts has been met. (elastic#80681) ...
Extracted from #81698
When adding a new set of functional tests that execute along with the normal CI groups some of the logic used to manage menu state started to fail because the timing was adjusted somewhat. I took a look and think the
MenuToggle
helper handled opening/closing menus by clicking a toggle button a little better than it is done today without making things much slower.I had originally implemented this in the
gis
PageObject but then once those tests started passing I noticed very similar logic in thetimePicker
PageObject failing as well. To make sure the fix was applied equally to both PageObjects I added aMenuToggle
service which provides a class that can be instantiated to provide.open()
and.close()
functions for a specific menu which can be reused within a service.@nreese it looks like you've worked on a lot of this code so if you wouldn't mind taking a look at the changes I'd appreciate it.