Skip to content

feat(table): core table unification #1354

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

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

Conversation

L2D2Grafana
Copy link
Collaborator

#1286

  • Removed drawer
  • Added resizable left sidebar, matching core grafana
  • Added a collapse button for small
Screen.Recording.2025-06-20.at.12.16.41.PM.mov

@L2D2Grafana L2D2Grafana force-pushed the l2d2/1286-core-table-unification branch from 4f7262c to 4e8bd80 Compare June 20, 2025 19:21
Copy link
Contributor

github-actions bot commented Jun 20, 2025

Bundle Size Changes

Hello! 👋 This comment was generated by a Github Action to help you and reviewers understand the impact of your PR on frontend bundle sizes.

Whenever this PR is updated, this comment will update to reflect the latest changes.

EntryPoint Size % Diff
module 80.19 KB (-59 Bytes) -0.07%
Files Total bundle size % Diff
29 9.17 MB (+11.13 KB) +0.12%
View detailed bundle information

Added

Name Size % Diff
267.js?_cache=c80189326055049322cf 675.18 KB (+675.18 KB) -
328.js?_cache=f17b60c3033b8ef0ff93 424.64 KB (+424.64 KB) -
470.js?_cache=29dd26bce42815d67980 161.22 KB (+161.22 KB) -
944.js?_cache=c9770b9500ce5eb4bbe7 11.11 KB (+11.11 KB) -
854.js?_cache=9da793b3efc18875808d 10.41 KB (+10.41 KB) -
826.js?_cache=91e39090c5611938563c 5.25 KB (+5.25 KB) -
677.js?_cache=056ebfc02700dd0a2614 2.42 KB (+2.42 KB) -
546.js?_cache=8e0beb2f22d2cbf6b122 1.24 KB (+1.24 KB) -
919.js?_cache=c4d71551d985ac2d78ea 783 Bytes (+783 Bytes) -
82.js?_cache=a68ad5c8663329786856 511 Bytes (+511 Bytes) -

Removed

Name Size % Diff
83.js?_cache=32c77be7b83289057f95 0 Bytes (-659.29 KB) -100.00%
328.js?_cache=f17339d9c32a0794110a 0 Bytes (-429.31 KB) -100.00%
470.js?_cache=3204c8fc47e32152da72 0 Bytes (-161.06 KB) -100.00%
944.js?_cache=f71d4a80dffcd450b1ad 0 Bytes (-11.12 KB) -100.00%
854.js?_cache=dfc3af049daeccb14055 0 Bytes (-10.42 KB) -100.00%
826.js?_cache=cf6d8a568b6c1fb2f511 0 Bytes (-5.4 KB) -100.00%
677.js?_cache=355db792dad45d10399a 0 Bytes (-2.44 KB) -100.00%
546.js?_cache=6f5ddc8aa7794d51d98a 0 Bytes (-1.24 KB) -100.00%
919.js?_cache=7be6ddd4150bd05cd8b5 0 Bytes (-785 Bytes) -100.00%
82.js?_cache=8aeb0e8d14bf6a95b54f 0 Bytes (-512 Bytes) -100.00%

Bigger

No assets were bigger

Smaller

Name Size % Diff
module.js 80.19 KB (-59 Bytes) -0.07%
View module information

Added

Name Size % Diff
re-resizable 35.15 KB (+35.15 KB) -
clsx 397 Bytes (+397 Bytes) -

Removed

No modules were removed

Bigger

Name Size % Diff
@grafana/scenes 727.39 KB (+3.58 KB) +0.49%
./Components/Table/Table.tsx 21.9 KB (+2.78 KB) +14.54%
i18next-browser-languagedetector 13.99 KB (+1.74 KB) +14.18%
semver 63.13 KB (+704 Bytes) +1.10%
react-i18next 26.47 KB (+607 Bytes) +2.29%
./Components/Table/ColumnSelection/LogsColumnSearch.tsx 2.71 KB (+524 Bytes) +23.27%
@floating-ui/utils 10.18 KB (+519 Bytes) +5.24%
@floating-ui/core 35.31 KB (+472 Bytes) +1.32%
./Components/Table/LineActionIcons.tsx 3.94 KB (+372 Bytes) +10.16%
@floating-ui/dom 25.84 KB (+231 Bytes) +0.88%
react-scroll-sync 52.01 KB (+170 Bytes) +0.32%
./Components/IndexScene/IndexScene.tsx 31.76 KB (+96 Bytes) +0.30%
./module.tsx 2.96 KB (+44 Bytes) +1.47%
@floating-ui/react-dom 10.33 KB (+41 Bytes) +0.39%
../package.json 4.27 KB (+25 Bytes) +0.57%

Smaller

Name Size % Diff
./Components/Table/ColumnSelection/LogsTableNavField.tsx 3.41 KB (-3.36 KB) -49.66%
react-draggable 51.56 KB (-2.01 KB) -3.76%
./Components/ServiceScene/Breakdowns/FieldValuesBreakdownScene.tsx 20.47 KB (-776 Bytes) -3.57%
./Components/ServiceScene/LogsTableScene.tsx 12.42 KB (-775 Bytes) -5.74%
./services/TagKeysProviders.ts 8.01 KB (-715 Bytes) -8.02%
./Components/Table/LogsTableHeaderWrap.tsx 8.01 KB (-584 Bytes) -6.65%
./services/TagValuesProviders.ts 9.99 KB (-419 Bytes) -3.93%
./Components/ServiceScene/Breakdowns/LabelValuesBreakdownScene.tsx 22.91 KB (-388 Bytes) -1.63%
./Components/Panels/PanelMenu.tsx 18.04 KB (-356 Bytes) -1.89%
./Components/Table/Context/TableColumnsContext.tsx 9.54 KB (-345 Bytes) -3.41%
./Components/Table/DefaultCellComponent.tsx 3.84 KB (-338 Bytes) -7.92%
./services/keyboardShortcuts.ts 7.53 KB (-322 Bytes) -4.01%
./Components/AppConfig/AppConfig.tsx 10.26 KB (-295 Bytes) -2.73%
./Components/ServiceScene/LogListControls.tsx 4.38 KB (-261 Bytes) -5.49%
./services/datasource.ts 24.62 KB (-259 Bytes) -1.02%
./Components/IndexScene/ShareButtonScene.tsx 8.83 KB (-256 Bytes) -2.75%
./Components/Table/TableProvider.tsx 1.47 KB (-178 Bytes) -10.60%
./Components/Table/LogLinePill.tsx 6.82 KB (-170 Bytes) -2.38%
./services/scenes.ts 3.2 KB (-165 Bytes) -4.80%
./Components/ServiceScene/Breakdowns/Patterns/PatternNameLabel.tsx 7.65 KB (-155 Bytes) -1.94%
./Components/Table/TableWrap.tsx 13.15 KB (-136 Bytes) -1.00%
./Components/Table/LogLineCellComponent.tsx 6.76 KB (-126 Bytes) -1.79%
./Components/ServiceScene/Breakdowns/LineFilterInput.tsx 5.56 KB (-121 Bytes) -2.08%
./Components/Table/ColumnSelection/ColumnSelectionDrawerWrap.tsx 8.18 KB (-57 Bytes) -0.68%
@tanstack/virtual-core 27.93 KB (-40 Bytes) -0.14%
./Components/ServiceScene/Breakdowns/Patterns/PatternsFrameScene.tsx 13.14 KB (-28 Bytes) -0.21%

@L2D2Grafana L2D2Grafana marked this pull request as ready for review June 23, 2025 19:26
@L2D2Grafana L2D2Grafana requested a review from a team as a code owner June 23, 2025 19:26
@gtk-grafana
Copy link
Contributor

@L2D2Grafana the code looks good at a glance, I'll dig in more later this week, but I'm noticing that there are some conflicts with the sidebar from the new logs panel:
image
image

Issues on main/ don't fix in this PR:
The pill heights don't match in the line/body column vs the others any more, but I see that in main as well.
image

Also if you put a numeric column first the buttons get all messed up
image

@L2D2Grafana L2D2Grafana force-pushed the l2d2/1286-core-table-unification branch 2 times, most recently from 0848d57 to 3fb1bce Compare June 26, 2025 22:54
@L2D2Grafana
Copy link
Collaborator Author

Put back row-reverse to account for the panel controls
Screenshot 2025-06-26 at 2 10 40 PM
Updated Pill heights to be the same
Screenshot 2025-06-27 at 11 14 26 AM
Fix for 1st column numbers
Screenshot 2025-06-27 at 11 07 16 AM

@L2D2Grafana L2D2Grafana force-pushed the l2d2/1286-core-table-unification branch from f97ce05 to 0024530 Compare July 7, 2025 18:54
@gtk-grafana
Copy link
Contributor

gtk-grafana commented Jul 7, 2025

Looking better, a few nits:
When resizing a column the reset custom resize button takes up a bit too much space:
image
vs main:
image

Maybe we should replace those buttons with an IconButton, or just remove it since we added the column reset in the column headers as well.

Also the context menus are missing the "add column" button that's currently present in the line filter pills (but not in standard columns):
image

vs main:
image

Feature request for another PR:
image
Show labels/show raw log line button might need a disabled state if the Log Line column is not visualized.

Feature request for another PR:
Looks like my hacky column width estimator isn't taking the new icons into effect, so we're seeing some columns with titles that are longer then the values push the icons out of the viewport:
image
Although maybe we want to address that after ng-table switch, I think wrapping text in the header was just added (although maybe the icons should overlap or push out the title, but those custom headers are pretty tough to work with)

@gtk-grafana
Copy link
Contributor

gtk-grafana commented Jul 7, 2025

Also when long labels get overflow: ellipsis the sampled cardinality (n values) wrap on two lines which looks kinda funky
image

@L2D2Grafana L2D2Grafana force-pushed the l2d2/1286-core-table-unification branch from 0024530 to 44a22aa Compare July 8, 2025 23:54
@L2D2Grafana
Copy link
Collaborator Author

  • Restored add column 😶‍🌫️
  • Removed reset width buttons
  • No wrap on sampled cardinality
    Screenshot 2025-07-08 at 4 51 09 PM

Todo:
Show labels/show raw log line button might need a disabled state if the Log Line column is not visualized.
Column title width with the new controls bar

@gtk-grafana gtk-grafana added this to the 1.0.23 milestone Jul 9, 2025
zIndex: 10,
}),
rzHandle: css({
[theme.transitions.handleMotion('no-preference', 'reduce')]: {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think for background transitions we don't need to respect the reduced motion unless something is actively moving, color/opacity transitions should be fine?

background: theme.colors.secondary.main,
borderRadius: theme.shape.radius.pill,
cursor: 'grab',
height: '50% !important',
Copy link
Contributor

Choose a reason for hiding this comment

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

If there's an !important I'd like to see a comment explaining why there is no other solution available

Copy link
Contributor

@gtk-grafana gtk-grafana left a comment

Choose a reason for hiding this comment

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

Everything looks good and all of the issues I saw earlier appear to be working as expected! 🎉 🎉 🎉 🎉

I did find another issue that is also on main, where adding filters that already exist from the table does not actually toggle, but keeps adding duplicate filters.

image

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