-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Code] Code Integrator Component #47180
Conversation
Pinging @elastic/code (Team:Code) |
@mw-ding here are the remaining items (or things to defer until APM has bandwidth) as I see them:
Also, |
💚 Build Succeeded
|
const handleSelect = (codeId: string) => { | ||
onRepoSelect(codeId); | ||
setShowSelector(false); | ||
// TODO: show success |
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.
are we going to implement the success notification in this PR. do you think it's currently blocked by the case that we haven't decided on if we should issue the REST API inside the component or not?
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.
Currently, the import calls are stubbed out, but the components are written under the assumption that they'll be making the calls themselves.
We could certainly allow the consumer to make the call themselves with a simpler callback, but then CodeIntegrator's not doing much besides generating HTML.
We should discuss both approaches with APM and go with whatever they think is best, but I think both will need the simpler component that is callback-agnostic.
💚 Build Succeeded
|
It's great to have some follow up items. Let's have a chat on them. But not quite necessary for this PR. |
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.
overall, LGTM. Let get this checked in first. @daveyholler can you take a look on the design's end?
please remember to flip the flag here if you want to try it out locally. https://github.com/elastic/kibana/pull/47180/files#diff-95a72659e4d8aff5a859fc873de1b710R54
x-pack/legacy/plugins/code/public/components/integrations/integrations.scss
Outdated
Show resolved
Hide resolved
💚 Build Succeeded
|
💚 Build Succeeded
|
* Abstracts shared ImportModal component * Uses placeholder data and callbacks in lieu of real integration
Integrations is too generic.
This got lost in the rebase, whoops.
💚 Build Succeeded |
… into console-token-iterator * 'console-token-iterator' of github.com:jloleysens/kibana: (184 commits) [functional/services] update webdriver lib and types (elastic#47381) Standardizing IconField implementation across the app (elastic#47196) Move ui/value_suggestions ⇒ NP data plugin (elastic#45762) Remove ui/persisted_log - Part 2 (elastic#47236) Update gulp related packages (elastic#47421) Update dependency idx to ^2.5.6 (elastic#47399) try running fewer jobs in parallel on the same worker (elastic#47403) Update webpack related packages (elastic#47402) Update jsonwebtoken related packages (elastic#47400) Update gulp related packages (major) (elastic#46665) Update dependency prettier to ^1.18.2 (elastic#47340) Update dependency @types/puppeteer to ^1.20.1 (elastic#47339) Update dependency @elastic/elasticsearch to ^7.4.0 (elastic#47338) Update dependency tar-fs to ^1.16.3 (elastic#47341) [Code] Code Integrator Component (elastic#47180) [Canvas][i18n] Sidebar (elastic#46090) Generate uuid in task Manager as Kibana uuid may not yet have been initialised [Code] Embedded Code Snippet Component (elastic#47183) Revert "Add pipeline for flaky test runner job (elastic#46740)" SearchSource: fix docvalue_fields and fields intersection logic (elastic#46724) ...
* Add CodeIntegrator component * Abstracts shared ImportModal component * Uses placeholder data and callbacks in lieu of real integration
Summary
This is a followup to #46402. It addresses part of elastic/code#1637. It is based upon #47183. As such, here is the diff for just this branch.
CodeIntegrator
component that allows consumers to either select a repo or import a new one, with appropriate callback propsChecklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.Documentation was added for features that require explanation or tutorialsUnit or functional tests were updated or added to match the most common scenariosThis was checked for keyboard-only and screenreader accessibilityFor maintainers