Skip to content

Conversation

@spalger
Copy link
Contributor

@spalger spalger commented Oct 27, 2020

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 the timePicker PageObject failing as well. To make sure the fix was applied equally to both PageObjects I added a MenuToggle 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.

@spalger spalger added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v7.11.0 labels Oct 27, 2020
@spalger spalger marked this pull request as ready for review October 27, 2020 01:02
@spalger spalger requested review from a team as code owners October 27, 2020 01:02
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@spalger spalger requested a review from nreese October 27, 2020 02:38
Copy link
Contributor

@nreese nreese left a 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

Copy link
Member

@dmlemeshko dmlemeshko left a 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.

@spalger spalger merged commit 441890c into elastic:master Oct 27, 2020
@spalger spalger deleted the implement/ftr-menu-toggle-helper branch October 27, 2020 16:09
spalger added a commit to spalger/kibana that referenced this pull request Oct 27, 2020
…astic#81709)

Co-authored-by: spalger <[email protected]>
# Conflicts:
#	test/functional/services/common/test_subjects.ts
spalger added a commit to spalger/kibana that referenced this pull request Oct 27, 2020
…astic#81709)

Co-authored-by: spalger <[email protected]>
# Conflicts:
#	test/functional/services/common/test_subjects.ts
spalger added a commit that referenced this pull request Oct 27, 2020
spalger added a commit that referenced this pull request Oct 27, 2020
…ng (#81709) (#81800)

Co-authored-by: spalger <[email protected]>
# Conflicts:
#	test/functional/services/common/test_subjects.ts
gmmorris added a commit to gmmorris/kibana that referenced this pull request Oct 27, 2020
* 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)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v7.11.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants