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] Name and description flyout for connectors #143827

Merged
merged 8 commits into from
Nov 15, 2022
Merged

[Enterprise Search] Name and description flyout for connectors #143827

merged 8 commits into from
Nov 15, 2022

Conversation

byronhulcher
Copy link
Contributor

@byronhulcher byronhulcher commented Oct 23, 2022 •
edited
Loading

Summary

Summarize your PR. If it involves visual changes include a screenshot or gif.

Checklist

Delete any items that are not applicable to this PR.

@byronhulcher byronhulcher added release_note:skip Skip the PR/issue when compiling release notes Team:EnterpriseSearch labels Oct 23, 2022
@byronhulcher byronhulcher self-assigned this Oct 23, 2022
@byronhulcher byronhulcher marked this pull request as ready for review October 25, 2022 20:40
@byronhulcher byronhulcher requested a review from a team October 25, 2022 20:40
@byronhulcher
Copy link
Contributor Author

@elasticmachine merge upstream

@byronhulcher byronhulcher changed the title [WIP] [Enterprise Search] Name and description flyout for connectors [Enterprise Search] Name and description flyout for connectors Oct 27, 2022
sphilipse
sphilipse reviewed Oct 27, 2022
View reviewed changes
Copy link
Member

@sphilipse sphilipse left a comment

Choose a reason for hiding this comment

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

This looks good, but one question: if users navigate away they just lose their changes. Do we want a confirmation dialog for that case?

...rch_index/connector/connector_name_and_description/connector_name_and_description_flyout.tsx Outdated
<EuiFlexItem grow={false}>
<EuiButtonEmpty
onClick={() => setIsEditing(false)}
isDisabled={status === Status.LOADING}
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: shouldn't this be isLoading?

@byronhulcher
Copy link
Contributor Author

if users navigate away they just lose their changes. Do we want a confirmation dialog for that case?

I think its fine as-is, but I could definitely add that as an enhancement when I'm back.

@byronhulcher byronhulcher enabled auto-merge (squash) October 28, 2022 18:19
@byronhulcher
Copy link
Contributor Author

Ugh I forgot about this after I went on PTO I'll clean up the conflicts and get it merged ASAP

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
enterpriseSearch 1843 1845 +2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
enterpriseSearch 2.0MB 2.0MB +1.7KB
Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 59 65 +6
osquery 108 113 +5
securitySolution 441 447 +6
total +19

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 67 73 +6
osquery 109 115 +6
securitySolution 518 524 +6
total +20

History

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

cc @byronhulcher

@byronhulcher byronhulcher merged commit 8974e82 into elastic:main Nov 15, 2022
@kibanamachine kibanamachine added v8.6.0 backport:skip This commit does not require backporting labels Nov 15, 2022
jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 15, 2022
@jloleysens
Merge branch 'main' into files-management
6c5fb8a 
* main: (65 commits)
  Migrate server-side `Root` and `Server` to packages (elastic#144990)
  [Discover] Handle no data views state for `esQuery` alert (elastic#145052)
  [ML] Allow updates for number of allocations and priority for trained model deployments (elastic#144704)
  [api-docs] 2022-11-15 Daily api_docs build (elastic#145203)
  [Security solution] remove guided onboarding feature flag (elastic#144247)
  [DOCS] Automate final case APIs (elastic#145007)
  [Enterprise Search] Name and description flyout for connectors (elastic#143827)
  [Guided onboarding] Update header button logic (elastic#144634)
  [Lens] Multi metric partition charts (elastic#143966)
  [Dashboard] [Controls] Add unmapped runtime field support to options list (elastic#144947)
  [Security Solution] Add Task Metric Collection to New Tasks (elastic#145181)
  [TriggersActionsUi] disable jest config in CI (elastic#145186)
  [TableListView] Enhance tag filtering (elastic#142108)
  [Cloud Posture] Compliance by CIS section table (elastic#145114)
  [8.6][Session View] Fix hidden alert flyout  in session view (elastic#145141)
  [customIntegrations] async load all components (elastic#145166)
  Fix time for logs smoke tests in integration test (elastic#145130)
  [RAM] Update rule status (elastic#140882)
  Update babel (main) (elastic#145060)
  [Actionable Observability] Add context.alertDetailsUrl variable to action connector template for APM rule types (elastic#144791)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sphilipse sphilipse sphilipse approved these changes

Assignees

@byronhulcher byronhulcher

Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:EnterpriseSearch v8.6.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@byronhulcher @kibana-ci @sphilipse @kibanamachine