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.

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

ui: statement details import #58701

Merged
merged 2 commits into from
Jan 19, 2021

Conversation

koorosh
Copy link
Collaborator

@koorosh koorosh commented Jan 11, 2021

Related to #57372
Depends on cockroachdb/ui#162
Depends on cockroachdb/yarn-vendored#51

Statement details and Diagnostics view pages are moved out
to admin-ui-components package and now they are imported back
as a reusable components.

In addition to changed imports, StatementDetails component now exposes
additional props for Diagnostics view tab. Before, StatementDetails
component used Diagnostics view connected to store and now it uses
pure component. This change were made to provide single point of
integration to redux store (the same way as other components connected).

Tracking analytics functionality for Diagnostics view was initially called directly
from components and now it's refactored out of component and involved
with redux actions and sagas. It was necessary to exclude this logic from pure
components and allow client apps which integrate Statement details to implement
their own logic for tracking analytics if necessary.

TODO:

  • remove all exported components from this code base to avoid duplicates.
  • update admin-ui-components package version in package.json

Release note: None

@koorosh koorosh requested a review from a team January 11, 2021 13:11
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@dhartunian dhartunian left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 8 of 8 files at r1, 10 of 10 files at r2.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@koorosh koorosh force-pushed the ui-statement-details-import branch 2 times, most recently from 932dadf to 0eda739 Compare January 13, 2021 12:42
@koorosh
Copy link
Collaborator Author

koorosh commented Jan 13, 2021

Lint errors can be fixed with this change cockroachdb/ui#172

@koorosh koorosh force-pushed the ui-statement-details-import branch from 0eda739 to e0d1b95 Compare January 14, 2021 13:57
Statement details and Diagnostics view pages are moved out
to `admin-ui-components` package and now they are imported back
as a reusable components.

In addition to changed imports, StatementDetails component now exposes
additional props for Diagnostics view tab. Before, StatementDetails
component used Diagnostics view connected to store and now it uses
pure component. This change were made to provide single point of
integration to redux store (the same way as other components connected).

Release note: None
Currently, many components were exported to `admin-ui-components`
package and these components weren't removed from this repo.
To avoid duplicates, these components removed and re-imported back.

Release note: None
@koorosh koorosh force-pushed the ui-statement-details-import branch from e0d1b95 to 724cf52 Compare January 18, 2021 11:08
@koorosh
Copy link
Collaborator Author

koorosh commented Jan 19, 2021

bors r+

@craig
Copy link
Contributor

craig bot commented Jan 19, 2021

Build succeeded:

@craig craig bot merged commit d84e680 into cockroachdb:master Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants