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

✨ [extension] add columns to the event list #2372

Merged
merged 6 commits into from Sep 15, 2023

Conversation

BenoitZugmeyer
Copy link
Member

@BenoitZugmeyer BenoitZugmeyer commented Aug 8, 2023

Motivation

When adding a new field or investigating a specific field value, it is a bit annoying to expand it
on every event.

Changes

  • Use a grid instead of a table for event list (this was not strictly needed but I find it more predictable even if it has a few issues)
  • Add support for columns in the event list to display some arbitrary fields.
    • Add columns by clicking on the json fields or by using the "Add column" button
    • Reorder columns by drag and dropping
    • Remove columns by dragging them out of the header

Screenshot 2023-08-24 at 12 25 21

Note: see, the space between the facet list and the event list is back!

Testing

  • Local
  • Staging
  • Unit
  • End to end

I have gone over the contributing documentation.

Copy link
Member Author

BenoitZugmeyer commented Aug 8, 2023

@BenoitZugmeyer BenoitZugmeyer changed the title ✨ add columns to the event list ✨ [extension] add columns to the event list Aug 8, 2023
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/extension--enhance-json-visualization branch from d082dc8 to cd350a9 Compare August 8, 2023 13:57
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/extension--enhance-json-visualization branch from cd350a9 to 16093c9 Compare August 23, 2023 14:38
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/extension--enhance-json-visualization branch from 16093c9 to 745b6df Compare August 23, 2023 14:47
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/extension--enhance-json-visualization branch from 745b6df to d784748 Compare August 23, 2023 15:49
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/extension--columns branch 2 times, most recently from 9ff9536 to b673acd Compare August 24, 2023 10:33
@BenoitZugmeyer BenoitZugmeyer marked this pull request as ready for review August 24, 2023 10:34
@BenoitZugmeyer BenoitZugmeyer requested a review from a team as a code owner August 24, 2023 10:34
Comment on lines -10 to -11
<Table striped verticalSpacing="xs">
<tbody>
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ question: ‏Why move from a table to a grid?

Copy link
Member Author

Choose a reason for hiding this comment

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

I reverted back to a table, the grid offers more control over the exact size of columns, but was too much trouble to implement. I want to revisit this one day when I have time :)

@bcaudan
Copy link
Contributor

bcaudan commented Aug 25, 2023

After playing with the UI:

🐛 You can "add column" on an already added column which create a new column with empty values

Screenshot 2023-08-25 at 14 04 59
Screenshot 2023-08-25 at 14 05 09

🐛 the display does not seem to handle well long URL in the message, maybe truncating them?

Screenshot 2023-08-25 at 14 13 38

💬 for removing a column, drag and drop outside of the header seems quite unusual.
What about adding a dropdown on column header with remove column (move left / right could also be added)?

💬 to indicate the columns can be reorderd by drag and drop, what about displaying "move" cursor on hover?

@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/extension--enhance-json-visualization branch from d784748 to e3e5fbb Compare September 7, 2023 13:10
@BenoitZugmeyer
Copy link
Member Author

BenoitZugmeyer commented Sep 7, 2023

@codecov-commenter
Copy link

codecov-commenter commented Sep 7, 2023

Codecov Report

Merging #2372 (1ce9dd9) into main (bd5aa77) will decrease coverage by 0.08%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##             main    #2372      +/-   ##
==========================================
- Coverage   93.47%   93.40%   -0.08%     
==========================================
  Files         226      226              
  Lines        6617     6622       +5     
  Branches     1470     1470              
==========================================
  Hits         6185     6185              
- Misses        432      437       +5     
Files Changed Coverage Δ
...tension/src/panel/hooks/useEvents/facetRegistry.ts 55.73% <33.33%> (-3.20%) ⬇️

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@BenoitZugmeyer
Copy link
Member Author

BenoitZugmeyer commented Sep 8, 2023

After playing with the UI:

🐛 You can "add column" on an already added column which create a new column with empty values

Fixed

🐛 the display does not seem to handle well long URL in the message, maybe truncating them?

Fixed by reverting to using a Table, so now long strings should wrap properly

💬 for removing a column, drag and drop outside of the header seems quite unusual. What about adding a dropdown on column header with remove column (move left / right could also be added)?

Added a button to remove columns.

💬 to indicate the columns can be reorderd by drag and drop, what about displaying "move" cursor on hover?

Native devtools tabs can be reordered by drag and drop, but don't have a "move" cursor. I think it could be best to follow the same UI, so I didn't change the cursor. I will revisit the drag behavior later to make it closer to the native devtools experience.

@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/extension--enhance-json-visualization branch from e3e5fbb to 31cf3e8 Compare September 8, 2023 12:37
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/extension--enhance-json-visualization branch from 31cf3e8 to 64e6b66 Compare September 15, 2023 08:25
Base automatically changed from benoit/extension--enhance-json-visualization to main September 15, 2023 09:23
@BenoitZugmeyer BenoitZugmeyer merged commit 26d48f7 into main Sep 15, 2023
17 checks passed
@BenoitZugmeyer BenoitZugmeyer deleted the benoit/extension--columns branch September 15, 2023 09:59
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.

None yet

4 participants