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

implements explorer data grid column reordering #1221

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

paulstn
Copy link
Collaborator

@paulstn paulstn commented Nov 1, 2023

Description

Implements data grid column reordering within the column toolbar drag and drop. Also implements it within the sidebar fields drag and drop, where the order of the selected fields can be manipulated but the available fields will still be sorted alphabetically.

Issues Resolved

This PR also resolves an issue within app analytics where the sidebar isn't populated with any fields.

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link

codecov bot commented Nov 1, 2023

Codecov Report

Merging #1221 (e21b6ab) into main (e0155e8) will decrease coverage by 0.03%.
Report is 6 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1221      +/-   ##
==========================================
- Coverage   44.07%   44.05%   -0.03%     
==========================================
  Files         329      329              
  Lines       19668    19690      +22     
  Branches     4735     4690      -45     
==========================================
+ Hits         8669     8674       +5     
- Misses      10422    10971     +549     
+ Partials      577       45     -532     
Flag Coverage Δ
dashboards-observability 44.05% <ø> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 88 files with indirect coverage changes

Signed-off-by: Paul Sebastian <[email protected]>
@@ -262,7 +283,7 @@ export function DataGrid(props: DataGridProps) {
toolbarVisibility={{
showColumnSelector: {
allowHide: false,
allowReorder: true,
allowReorder: explorerFields.selectedFields.length > 0,
Copy link
Collaborator

@mengweieric mengweieric Nov 1, 2023

Choose a reason for hiding this comment

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

If by default only have timestamp and _source for initial state, then there's no selected field explorerFields.selectedFields, but datagrid should still allow user to reorder timnestamp and _source is it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here i'm actually disabling reordering when no fields are selected, so that no reordering is done when the default columns appear in data grid. The reason I do this is because the default columns are 'artificial fields' and I don't want them to appear as objects in the sidebar.

Copy link
Collaborator

Choose a reason for hiding this comment

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

then it doesn't match with existing behavior in discover

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

existing behavior is discover is bugged a little bit, the default columns don't get reordered when done through the data grid even though the names move and the sidebar also has some inconsistencies. I extrapolated that since the default columns don't actually move themselves, then I should disable the ability to attempt at doing so.

public/components/event_analytics/utils/utils.tsx Outdated Show resolved Hide resolved
Signed-off-by: Paul Sebastian <[email protected]>
@paulstn paulstn assigned paulstn and unassigned paulstn Nov 2, 2023
@paulstn paulstn force-pushed the explorer-data-grid-column-reordering branch 2 times, most recently from e87712d to bcf7cb0 Compare November 2, 2023 17:23
Signed-off-by: Paul Sebastian <[email protected]>
Signed-off-by: Paul Sebastian <[email protected]>
@mengweieric
Copy link
Collaborator

@paulstn please resolve the conflicts

RyanL1997 added a commit to RyanL1997/dashboards-observability that referenced this pull request Apr 18, 2024
* Increment Version to 2.5.0 and add web driver for selenium test case

Signed-off-by: Ryan Liang <[email protected]>
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.

2 participants