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.

Sign up for GitHub

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

Jump to bottom

[Enterprise Search] Move renderHeaderActions back into mount useEffect #80861

Merged
merged 1 commit into from
Oct 16, 2020
Merged

[Enterprise Search] Move renderHeaderActions back into mount useEffect #80861

merged 1 commit into from
Oct 16, 2020

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented Oct 16, 2020 •
edited by scottybollinger
Loading

Summary

We originally moved renderHeaderActions() outside of useEffect (3254c62) because it was disappearing between in-app navigation within Workplace Search. However, it turns out this was an issue with Kibana's internal navigateToApp API which has now been fixed (#80809).

We can now move renderHeaderActions() back into our on-mount useEffect, which incidentally also solves the issue of our header actions not correctly unmounting when navigating outside the Workplace Search app.

QA

  • Navigate directly to http://localhost:5601/app/enterprise_search/workplace_search/groups
  • Confirm that navigating within WS (groups, group overview, source prioritization) via links, breadcrumbs, nav & side nav does not cause the Header Action menu to disappear
  • Navigate from Workplace Search to Enterprise Search via breadcrumbs. Confirm the header correctly disappears
  • Navigate from Workplace Search to the Maps plugin via side nav. Confirm the header correctly renders Maps' header actions
  • Test the above on both dev yarn start and 'prod' (yarn start --no-base-path)

Checklist

@cee-chen cee-chen added Feature:Plugins release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v7.11.0 labels Oct 16, 2020
@cee-chen cee-chen requested review from scottybollinger and a team October 16, 2020 16:00
@cee-chen
Copy link
Member Author

@scottybollinger Thanks to Pierre's awesome work, this fix is now super easy on our end. Would you mind re-testing my QA steps in the PR description so that we can confirm it's working for more than just one person?

scottybollinger
scottybollinger approved these changes Oct 16, 2020
View reviewed changes
Copy link
Contributor

@scottybollinger scottybollinger left a comment

Choose a reason for hiding this comment

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

LGTM and all QA works as outlined 💯

@cee-chen
Copy link
Member Author

Hooray! Thanks Scotty!

@cee-chen cee-chen added the bug Fixes for quality problems that affect the customer experience label Oct 16, 2020
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

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

@cee-chen cee-chen merged commit 02d850f into elastic:master Oct 16, 2020
@cee-chen cee-chen deleted the header-actions branch October 16, 2020 17:47
cee-chen pushed a commit to cee-chen/kibana that referenced this pull request Oct 16, 2020
@cee-chen
Move renderHeaderActions back into mount useEffect + update tests (el…
fcbc3c0 
…astic#80861)
@cee-chen cee-chen mentioned this pull request Oct 16, 2020
Merged
cee-chen pushed a commit to cee-chen/kibana that referenced this pull request Oct 16, 2020
@cee-chen
Move renderHeaderActions back into mount useEffect + update tests (el…
52d88a8 
…astic#80861)
@cee-chen cee-chen mentioned this pull request Oct 16, 2020
Merged
cee-chen pushed a commit that referenced this pull request Oct 16, 2020
Move renderHeaderActions back into mount useEffect + update tests (#8…
727b625 
…0861) (#80886)
cee-chen pushed a commit that referenced this pull request Oct 16, 2020
Move renderHeaderActions back into mount useEffect + update tests (#8…
0fefa6f 
…0861) (#80885)
gmmorris added a commit to gmmorris/kibana that referenced this pull request Oct 19, 2020
@gmmorris
Merge branch 'master' into task-manager/refactor-oversized-class
c30d71b 
* master: (43 commits)
  [ML] Transforms: Fix tab ids for expanded row. (elastic#80666)
  server logs config paths to use for runner (elastic#52980)
  Fix audit logger logging to console even when disabled (elastic#80928)
  skip flaky suite (elastic#80929)
  Added Enterprise Search config to kibana-docker (elastic#80872)
  skip flaky suite (elastic#80914)
  [keystore_cli] parse values as JSON before adding to keystore (elastic#80848)
  [Ingest Manager] Fix for comparing versions with -SNAPSHOT suffix (elastic#80742)
  ECS audit logging (elastic#74640)
  [Uptime] Add client-side unit tests for remaining synthetics code (elastic#80215)
  [Security_Solution][Resolver] Promote z-index on node labels (elastic#80854)
  Move renderHeaderActions back into mount useEffect + update tests (elastic#80861)
  [Reporting] Document Network Policy configuration (elastic#80431)
  [Reporting] Add contextual documentation for CSV Max Bytes setting (elastic#80782)
  Add catch for Enterprise Search sending back a 401 response instead of redirect (elastic#80757)
  [Actions] Back Button on Add Connector Flyout (elastic#80160)
  removing `kibana_datatable` in favor of `datatable`  (elastic#80548)
  [Alerting UI] Updating 'Add new' wording (elastic#80509)
  [Docs] Document Encrypted Saved Objects functionality. (elastic#80183)
  [Discover] fix auto-refresh (elastic#80635)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Oct 19, 2020
@gmmorris
Merge branch 'master' into task-manager/health
12e52e6 
* master: (23 commits)
  [ML] Transforms: Fix tab ids for expanded row. (elastic#80666)
  server logs config paths to use for runner (elastic#52980)
  Fix audit logger logging to console even when disabled (elastic#80928)
  skip flaky suite (elastic#80929)
  Added Enterprise Search config to kibana-docker (elastic#80872)
  skip flaky suite (elastic#80914)
  [keystore_cli] parse values as JSON before adding to keystore (elastic#80848)
  [Ingest Manager] Fix for comparing versions with -SNAPSHOT suffix (elastic#80742)
  ECS audit logging (elastic#74640)
  [Uptime] Add client-side unit tests for remaining synthetics code (elastic#80215)
  [Security_Solution][Resolver] Promote z-index on node labels (elastic#80854)
  Move renderHeaderActions back into mount useEffect + update tests (elastic#80861)
  [Reporting] Document Network Policy configuration (elastic#80431)
  [Reporting] Add contextual documentation for CSV Max Bytes setting (elastic#80782)
  Add catch for Enterprise Search sending back a 401 response instead of redirect (elastic#80757)
  [Actions] Back Button on Add Connector Flyout (elastic#80160)
  removing `kibana_datatable` in favor of `datatable`  (elastic#80548)
  [Alerting UI] Updating 'Add new' wording (elastic#80509)
  [Docs] Document Encrypted Saved Objects functionality. (elastic#80183)
  [Discover] fix auto-refresh (elastic#80635)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scottybollinger scottybollinger scottybollinger approved these changes

Assignees
No one assigned
Labels
bug Fixes for quality problems that affect the customer experience Feature:Plugins release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v7.11.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cee-chen @kibanamachine @scottybollinger