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

[ftr/menuToggle] provide helper for enhanced menu toggle handling #81709

Merged
merged 1 commit into from
Oct 27, 2020

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 pushed a commit to spalger/kibana that referenced this pull request Oct 27, 2020
…astic#81709)

Co-authored-by: spalger <spalger@users.noreply.github.com>
# Conflicts:
#	test/functional/services/common/test_subjects.ts
spalger pushed a commit to spalger/kibana that referenced this pull request Oct 27, 2020
…astic#81709)

Co-authored-by: spalger <spalger@users.noreply.github.com>
# Conflicts:
#	test/functional/services/common/test_subjects.ts
spalger added a commit that referenced this pull request Oct 27, 2020
…ing (#81709) (#81801)

Co-authored-by: spalger <spalger@users.noreply.github.com>
spalger pushed a commit that referenced this pull request Oct 27, 2020
…ng (#81709) (#81800)

Co-authored-by: spalger <spalger@users.noreply.github.com>
# 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