-
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
turning it all into one React app #35185
Conversation
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.
@bmcconaghy this is awesome!
i didn't test locally yet. it looks like there are a couple files that may have been deleted accidentally.
confirm_watches_modal.tsx
form_errors.tsx
delete_watches_modal.tsx
@@ -0,0 +1,3 @@ | |||
<kbn-management-app section="elasticsearch/watcher"> | |||
<div id="watchReactRoot"></div> | |||
</kbn-management-app> |
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.
nit: no new line
@@ -0,0 +1,23 @@ | |||
/* |
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.
for my own knowledge... do we still need this now that we switched to react router?
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.
arguably not. probably should be using history for nav.
{selectedTab === WATCH_EDIT_TAB && ( | ||
<JsonWatchEditForm urlService={urlService} licenseService={licenseService} /> | ||
)} | ||
{selectedTab === WATCH_EDIT_TAB && <JsonWatchEditForm licenseService={licenseService} />} |
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.
rather than passing this in as a prop, could you just import LicenseServiceContext
directly from the JsonWatchEditForm
component?
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.
good point, fixed.
💔 Build Failed |
44f72f1
to
084a8bf
Compare
084a8bf
to
048b7e7
Compare
@alisonelizabeth thx for the comments, pushed fixes for those things. |
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.
LGTM
💔 Build Failed |
retest |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
retest |
retest |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
aeb2fbb
to
fc60e58
Compare
💔 Build Failed |
fc60e58
to
5237381
Compare
💔 Build Failed |
5237381
to
97a4a99
Compare
💚 Build Succeeded |
This deletes a lot of Angular code and turns it all into one big React app. I still need to address breadcrumbs and reducing the refetching of watches, but those will get addressed in a separate PR.