-
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
Deangularize Dashboard #82909
Deangularize Dashboard #82909
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.
ML changes LGTM
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.
SCSS changes look good.
Pinging @elastic/kibana-presentation (Team:Presentation) |
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.
This PR is quite dense. I feel like the difficulty in review could be mitigated by some code style changes and some encapsulation. At the same time, I don't want to over-complicate the remainder of our work on TTV.
I'd be content to have some preliminary fixes (destructuring, for example) and then follow up as we write tests. Thoughts?
src/plugins/dashboard/public/application/actions/replace_panel_action.tsx
Outdated
Show resolved
Hide resolved
src/plugins/dashboard/public/application/listing/dashboard_listing.tsx
Outdated
Show resolved
Hide resolved
src/plugins/dashboard/public/application/listing/dashboard_listing.tsx
Outdated
Show resolved
Hide resolved
src/plugins/dashboard/public/application/listing/dashboard_listing.tsx
Outdated
Show resolved
Hide resolved
src/plugins/dashboard/public/application/listing/dashboard_listing.tsx
Outdated
Show resolved
Hide resolved
src/plugins/dashboard/public/application/listing/dashboard_listing.tsx
Outdated
Show resolved
Hide resolved
8679154
to
1adda50
Compare
1adda50
to
068c9fa
Compare
… updated. Started moving more i18n into strings file
…eRenderer to render dashboard embeddable
213d081
to
3974639
Compare
⏳ Build in-progress, with failures
Failed CI StepsHistory
To update your PR or re-run it, just comment with: |
…tainer, removed lastContainerId
3974639
to
4c87f64
Compare
…hHistory state issue
💚 Build SucceededMetrics [docs]Async chunks
Distributable file count
Page load bundle
History
To update your PR or re-run it, just comment with: |
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.
I'm satisfied with the iterations and improvements, enough to get this checked in and move forward with tweaks, so long as you're confident in the test coverage.
This is a beast of an effort here, @ThomThomson ... incredible work!
Deangularized dashboard application
Summary
Closes #65614
This PR completely removes Angular from Dashboard, and re-structures the dashboard application into a React Functional Component.
There should be no visual or behavioural changes at all in this PR. Routing, loading, and parts of the state management outside of
dashboard_state_manager
have been totally replaced, so this PR could use some very thorough testing.Checklist
Delete any items that are not applicable to this PR.
For maintainers