You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "amaannawab923 (via GitHub)" <gi...@apache.org> on 2025/05/19 14:34:16 UTC
[PR] feat(viz-type): Ag grid table plugin Integration [superset]
amaannawab923 opened a new pull request, #33517:
URL: https://github.com/apache/superset/pull/33517
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
This pr integrates the basic functionality of ag grid table as a new viz plugin
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2096391619
##########
superset-frontend/packages/superset-ui-core/src/chart/types/VizType.ts:
##########
@@ -54,6 +54,7 @@ export enum VizType {
Step = 'echarts_timeseries_step',
Sunburst = 'sunburst_v2',
Table = 'table',
+ TableAgGrid = 'table_ag_grid',
Review Comment:
Hey @justinpark — happy to collaborate on this! 😊
I ran into a couple of issues with the AG Grid integration in SQL Lab that I wanted to highlight:
1. AG Grid styles not applying correctly 🎨
The default AG Grid table in SQL Lab doesn’t seem to reflect the standard AG Grid themes.
Here's a screenshot showing the current state:
<img width="1370" alt="Screenshot 2025-05-20 at 1 13 01 AM" src="https://github.com/user-attachments/assets/356cedf5-542c-43b2-bd19-d735cd993ca4" />
In comparison, the AG Grid demo (https://www.ag-grid.com/example-finance/) looks much more polished.
To fix this, I manually imported the AG Grid styles from the node_modules directory into a separate CSS file. After doing so, the table styling aligned much better with AG Grid’s intended design:
<img width="1192" alt="Screenshot 2025-05-20 at 1 11 51 AM" src="https://github.com/user-attachments/assets/8f85188c-fc90-4f5b-a5d8-c70018fd1f5b" />
2. Missing column filters due to custom headers 🚫🔍
Currently, it looks like we’re losing out on the built-in column filters because of our custom header implementation:
<img width="1509" alt="Screenshot 2025-05-20 at 1 15 28 AM" src="https://github.com/user-attachments/assets/247449cd-9bf8-4cc6-a9cd-dcb6a0d84bed" />
This can be resolved by implementing the column menu as an inner header component, as documented here:
https://www.ag-grid.com/javascript-data-grid/column-headers/#inner-header-component
That way, we can retain the native column filtering capabilities while customizing the header.
✅ If we address these two points, we should be in a solid position to standardize on a common AG Grid table component.
cc: @michael-s-molina
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2899107532
@eschutho Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/15171547045). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2970282530
@geido Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/15634884537). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "dkrat7 (via GitHub)" <gi...@apache.org>.
dkrat7 commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2983221373
This is awesome. I tested it a bit and it seems that text align is not working for string columns. It works for numbers - but only when cell bars are active.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2967054025
> > @michael-s-molina Would be taking up the whole table migration in stages , So this PR will focus on complete table Parity
> > And the follow up PR will take up Code deduplication for sqllab & Table V2 (Ag grid table) as the Testing efforts required are too much for complete parity with Superset table
>
> @amaannawab923 @geido Are we fully committed with these migrations? The worst thing that can happen is supporting another table in the codebase. This is reminding me of the multiple select versions (7) we had in the past and the huge effort we did to unify them. Currently, with this PR we'll have 4 different table implementations:
>
> ```
> - plugin-chart-table
> - plugin-chart-ag-grid-table
> - src/component/GridTable
> - src/components/Table
> ```
>
> cc @justinpark @villebro @mistercrunch @rusackas
The migration should be pretty straight forward from here , This Ag Grid table works with the same form data as Superset Table & works out pretty well with all of the features achieving complete parity , the Migration should be pretty easy going forward
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2166569285
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomPopover.tsx:
##########
@@ -0,0 +1,113 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { useEffect, useRef, useState, cloneElement } from 'react';
+import { styled } from '@superset-ui/core';
+
+const PopoverWrapper = styled.div`
+ position: relative;
+ display: inline-block;
+`;
+
+const PopoverContainer = styled.div`
+ ${({ theme }) => `
+ position: fixed;
+ background: ${theme.colors.grayscale.light4};
+ border: 1px solid ${theme.colors.grayscale.light2};
+ border-radius: ${theme.borderRadius}px;
+ box-shadow: 0 ${theme.sizeUnit / 2}px ${theme.sizeUnit * 2}px ${theme.colors.grayscale.light1}40;
Review Comment:
What does this Popover component do that requires a custom implementation? Can we use an Ant Design vanilla component for this so that we can avoid style customizations as well?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2167224275
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/styles/ag-grid.css:
##########
@@ -0,0 +1,6924 @@
+/* eslint-disable camelcase */
Review Comment:
I don't think this file is needed - we should use AG Grid's theming API instead
https://www.ag-grid.com/react-data-grid/themes/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "justinpark (via GitHub)" <gi...@apache.org>.
justinpark commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2102903712
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx:
##########
@@ -0,0 +1,228 @@
+/* eslint-disable camelcase */
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ useCallback,
+ useMemo,
+ useRef,
+ memo,
+ useState,
+ ChangeEvent,
+} from 'react';
+
+import {
+ AllCommunityModule,
+ ClientSideRowModelModule,
+ type ColDef,
+ ModuleRegistry,
+ GridReadyEvent,
+ GridApi,
+} from 'ag-grid-community';
+import { AgGridReact } from 'ag-grid-react';
+import './styles/ag-grid.css';
+
+import { type FunctionComponent } from 'react';
+
+import { styled, css } from '@superset-ui/core';
+
+export interface Props {
+ gridTheme?: string;
+ isDarkMode?: boolean;
+ gridHeight?: number | null;
+ updateInterval?: number;
+ data?: any[];
+ onGridReady?: (params: GridReadyEvent) => void;
+ colDefsFromProps: any[];
+ includeSearch: boolean;
+ allowRearrangeColumns: boolean;
+ pagination: boolean;
+ pageSize: number;
+}
+
+ModuleRegistry.registerModules([AllCommunityModule, ClientSideRowModelModule]);
+
+const StyledContainer = styled.div`
+ ${({ theme }) => css`
+ .input-container {
+ margin-left: auto;
+ }
+ .input-wrapper {
+ position: relative;
+ display: flex;
+ align-items: center;
+ overflow: visible;
+ margin-bottom: 1rem;
+ }
+
+ .input-wrapper svg {
+ pointer-events: none;
+ transform: translate(28px, 2px);
+ color: ${theme.colors.grayscale.base};
+ }
+
+ .input-wrapper input {
+ font-size: ${theme.typography.sizes.s}px;
+ padding: 0.375em 0.75em 0.375em 2.4em;
+ line-height: 1.4;
+ border-radius: ${theme.gridUnit}px;
+ border: 1px solid ${theme.colors.grayscale.light2};
+ background-color: transparent;
+ outline: none;
+
+ &:focus {
+ border-color: ${theme.colors.primary.base};
+ }
+
+ &::placeholder {
+ color: ${theme.colors.grayscale.light1};
+ }
+ }
+ `}
+`;
+
+// Memoize the entire component
+const AgGridDataTable: FunctionComponent<Props> = memo(
+ ({
+ gridTheme = 'ag-theme-quartz',
+ isDarkMode = false,
+ gridHeight = null,
+ data = [],
+ onGridReady,
+ colDefsFromProps,
+ includeSearch,
+ allowRearrangeColumns,
+ pagination,
+ pageSize,
+ }) => {
+ const gridRef = useRef<AgGridReact>(null);
+ const gridApiRef = useRef<GridApi | null>(null);
+
+ const defaultColDef = useMemo<ColDef>(
+ () => ({
+ flex: 1,
+ filter: true,
+ enableRowGroup: true,
+ enableValue: true,
+ sortable: true,
+ resizable: true,
+ minWidth: 100,
+ }),
+ [],
+ );
+
+ // Handle grid ready event
+ const handleGridReady = useCallback(
+ (params: GridReadyEvent) => {
+ gridApiRef.current = params.api;
+ if (onGridReady) {
+ onGridReady(params);
+ }
+
+ // Optional: Auto-size columns on initial load
+ params.api.sizeColumnsToFit();
+ },
+ [onGridReady],
+ );
+
+ // Memoize container style
+ const containerStyle = useMemo(
+ () => ({
+ height: gridHeight
+ ? includeSearch
+ ? `${gridHeight - 16}px`
+ : `${gridHeight}px`
+ : '100%',
+ width: '100%',
+ }),
+ [gridHeight],
+ );
+
+ // Memoize grid class name
+ const gridClassName = useMemo(
+ () => `ag-theme-quartz${isDarkMode ? '-dark' : ''}`,
+ [isDarkMode],
+ );
+
+ const [quickFilterText, setQuickFilterText] = useState<string>();
+ const onFilterTextBoxChanged = useCallback(
+ ({ target: { value } }: ChangeEvent<HTMLInputElement>) =>
+ setQuickFilterText(value),
+ [],
+ );
+
+ console.log(pageSize, 'page size');
+
+ return (
+ <StyledContainer>
+ <div className={gridClassName} style={containerStyle}>
+ {includeSearch && (
+ <div className="input-wrapper">
+ <div className="input-container">
+ <svg
+ width="16"
+ height="16"
+ viewBox="0 0 16 16"
+ fill="none"
+ xmlns="http://www.w3.org/2000/svg"
+ >
+ <path
+ fillRule="evenodd"
+ clipRule="evenodd"
+ d="M11.5014 7.00039C11.5014 7.59133 11.385 8.1765 11.1588 8.72246C10.9327 9.26843 10.6012 9.7645 10.1833 10.1824C9.76548 10.6002 9.2694 10.9317 8.72344 11.1578C8.17747 11.384 7.59231 11.5004 7.00136 11.5004C6.41041 11.5004 5.82525 11.384 5.27929 11.1578C4.73332 10.9317 4.23725 10.6002 3.81938 10.1824C3.40152 9.7645 3.07005 9.26843 2.8439 8.72246C2.61776 8.1765 2.50136 7.59133 2.50136 7.00039C2.50136 5.80691 2.97547 4.66232 3.81938 3.81841C4.6633 2.97449 5.80789 2.50039 7.00136 2.50039C8.19484 2.50039 9.33943 2.97449 10.1833 3.81841C11.0273 4.66232 11.5014 5.80691 11.5014 7.00039ZM10.6814 11.7404C9.47574 12.6764 7.95873 13.1177 6.43916 12.9745C4.91959 12.8314 3.51171 12.1145 2.50211 10.9698C1.49252 9.8251 0.957113 8.33868 1.0049 6.81314C1.05268 5.28759 1.68006 3.83759 2.75932 2.75834C3.83857 1.67908 5.28856 1.0517 6.81411 1.00392C8.33966 0.956136 9.82608 1.49154 10.9708 2.50114C12.1154 3.51073 12.8323 4.91862 12.9755 6.43819C13.1187 7.95775 12.6773 9.47476 11.7414
10.6804L14.5314 13.4704C14.605 13.539 14.6642 13.6218 14.7051 13.7138C14.7461 13.8058 14.7682 13.9052 14.77 14.0059C14.7717 14.1066 14.7532 14.2066 14.7155 14.3C14.6778 14.3934 14.6216 14.4782 14.5504 14.5494C14.4792 14.6206 14.3943 14.6768 14.301 14.7145C14.2076 14.7522 14.1075 14.7708 14.0068 14.769C13.9061 14.7672 13.8068 14.7452 13.7148 14.7042C13.6228 14.6632 13.54 14.6041 13.4714 14.5304L10.6814 11.7404Z"
+ fill="currentColor"
+ />
+ </svg>
+
+ <input
+ type="text"
+ id="filter-text-box"
+ placeholder="Search"
+ onInput={onFilterTextBoxChanged}
+ />
+ </div>
+ </div>
+ )}
+
+ <AgGridReact
Review Comment:
We already have an AgGrid component that is wrapped to match the superset theme. Instead of directly importing the AG Grid table, we recommend referencing and using `src/components/GridTable`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2967108758
> Migration should be pretty easy going forward
@amaannawab923 Just to be clear, there are multiple migrations:
1. Migrating `plugin-chart-table` to `plugin-chart-ag-grid-table`
2. Extracting common parts of `plugin-chart-ag-grid-table` and `GridTable` (used in SQL Lab) into core to avoid code duplication
3. Replace `src/components/Table` with `src/component/GridTable`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2949488625
@geido Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/15493228983). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2146300701
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomHeader.tsx:
##########
@@ -0,0 +1,346 @@
+/* eslint-disable jsx-a11y/no-static-element-interactions */
+/* eslint-disable theme-colors/no-literal-colors */
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { useRef, useState } from 'react';
+import { styled } from '@superset-ui/core';
+import {
+ ArrowDownOutlined,
+ ArrowUpOutlined,
+ PlusOutlined,
+} from '@ant-design/icons';
+import { IHeaderParams, Column, ColDef } from 'ag-grid-community';
+import CustomPopover from './CustomPopover';
+import { CustomColDef } from '..';
+
+const ThreeDots = ({ size = 14, color = 'black' }) => (
+ <svg
+ width={size}
+ height={size}
+ viewBox="0 0 16 16"
+ fill={color}
+ xmlns="http://www.w3.org/2000/svg"
+ >
+ <circle cx="8" cy="3" r="1.2" />
+ <circle cx="8" cy="8" r="1.2" />
+ <circle cx="8" cy="13" r="1.2" />
+ </svg>
+);
+const FilterIcon = (
+ <svg width="16" height="16" viewBox="0 0 24 24" fill="currentColor">
+ <rect x="3" y="6" width="18" height="2" rx="1" />
+ <rect x="6" y="11" width="12" height="2" rx="1" />
+ <rect x="9" y="16" width="6" height="2" rx="1" />
+ </svg>
+);
+
+// Styled Components
+const Container = styled.div`
+ display: flex;
+ width: 100%;
+
+ .three-dots-menu {
+ align-self: center;
+ margin-left: 5px;
+ cursor: pointer;
+ padding: 2px;
+ border-radius: 4px;
+ }
+`;
+
+const HeaderContainer = styled.div`
+ width: 100%;
+ display: flex;
+ align-items: center;
+ cursor: pointer;
+ padding: 0 8px;
Review Comment:
can we use the theme for all the css spacings in here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2899127144
@eschutho Ephemeral environment spinning up at http://44.248.10.69:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-3031127773
@geido Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/16043867472). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido merged PR #33517:
URL: https://github.com/apache/superset/pull/33517
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2166606123
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/Pagination.tsx:
##########
@@ -0,0 +1,216 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/* eslint-disable theme-colors/no-literal-colors */
+import { styled, t } from '@superset-ui/core';
+import {
+ VerticalLeftOutlined,
+ VerticalRightOutlined,
+ LeftOutlined,
+ RightOutlined,
+} from '@ant-design/icons';
+
+const PaginationContainer = styled.div`
Review Comment:
This pagination component was implemented for the sake of implementing Server Pagination on Community Module since server pagination isnt available on Community
I made 1 to 1 copy of the Client side pagination component of Ag Grid table ... since Ag Grid doesnt provide access to its components ... I created it from scratch
P.s I didnt know that ant design provided pagination ... Let me see if i can Use Ant design pagination
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2166606972
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/SearchSelectDropdown.tsx:
##########
@@ -0,0 +1,55 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/* eslint-disable import/no-extraneous-dependencies */
+import { styled } from '@superset-ui/core';
+import { Select } from 'antd';
Review Comment:
Yes yes ... Will do that ... This change was before the components were moved
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2979917437
@geido Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/15705434640). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2145015810
##########
superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts:
##########
@@ -58,6 +58,7 @@ export enum FeatureFlag {
SlackEnableAvatars = 'SLACK_ENABLE_AVATARS',
EnableDashboardScreenshotEndpoints = 'ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS',
EnableDashboardDownloadWebDriverScreenshot = 'ENABLE_DASHBOARD_DOWNLOAD_WEBDRIVER_SCREENSHOT',
+ TableV2TimeComparisonEnabled = 'TABLE_V2_TIME_COMPARISON_ENABLED',
Review Comment:
Yep
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2103032376
##########
superset-frontend/packages/superset-ui-core/src/chart/types/VizType.ts:
##########
@@ -54,6 +54,7 @@ export enum VizType {
Step = 'echarts_timeseries_step',
Sunburst = 'sunburst_v2',
Table = 'table',
+ TableAgGrid = 'table_ag_grid',
Review Comment:
It seems this component is well-written and doesn't have access to Superset's internal state directly like Redux calls. There's currently an effort from Preset to move many components from `src/components` to `@superset-ui/core` to be reusable by plugins and later by extensions. This seems like a perfect example of a valuable component that needs to be in `@superset-ui/core`. In case, there are any references to Redux or Superset's specific things, we just need to extract them from the component to a HOC and keep the component as a UI element before moving to `@superset-ui/core`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "justinpark (via GitHub)" <gi...@apache.org>.
justinpark commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2102906104
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx:
##########
@@ -0,0 +1,228 @@
+/* eslint-disable camelcase */
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ useCallback,
+ useMemo,
+ useRef,
+ memo,
+ useState,
+ ChangeEvent,
+} from 'react';
+
+import {
+ AllCommunityModule,
+ ClientSideRowModelModule,
+ type ColDef,
+ ModuleRegistry,
+ GridReadyEvent,
+ GridApi,
+} from 'ag-grid-community';
+import { AgGridReact } from 'ag-grid-react';
+import './styles/ag-grid.css';
+
+import { type FunctionComponent } from 'react';
+
+import { styled, css } from '@superset-ui/core';
+
+export interface Props {
+ gridTheme?: string;
+ isDarkMode?: boolean;
+ gridHeight?: number | null;
+ updateInterval?: number;
+ data?: any[];
+ onGridReady?: (params: GridReadyEvent) => void;
+ colDefsFromProps: any[];
+ includeSearch: boolean;
+ allowRearrangeColumns: boolean;
+ pagination: boolean;
+ pageSize: number;
+}
+
+ModuleRegistry.registerModules([AllCommunityModule, ClientSideRowModelModule]);
+
+const StyledContainer = styled.div`
+ ${({ theme }) => css`
+ .input-container {
+ margin-left: auto;
+ }
+ .input-wrapper {
+ position: relative;
+ display: flex;
+ align-items: center;
+ overflow: visible;
+ margin-bottom: 1rem;
+ }
+
+ .input-wrapper svg {
+ pointer-events: none;
+ transform: translate(28px, 2px);
+ color: ${theme.colors.grayscale.base};
+ }
+
+ .input-wrapper input {
+ font-size: ${theme.typography.sizes.s}px;
+ padding: 0.375em 0.75em 0.375em 2.4em;
+ line-height: 1.4;
+ border-radius: ${theme.gridUnit}px;
+ border: 1px solid ${theme.colors.grayscale.light2};
+ background-color: transparent;
+ outline: none;
+
+ &:focus {
+ border-color: ${theme.colors.primary.base};
+ }
+
+ &::placeholder {
+ color: ${theme.colors.grayscale.light1};
+ }
+ }
+ `}
+`;
+
+// Memoize the entire component
+const AgGridDataTable: FunctionComponent<Props> = memo(
+ ({
+ gridTheme = 'ag-theme-quartz',
+ isDarkMode = false,
+ gridHeight = null,
+ data = [],
+ onGridReady,
+ colDefsFromProps,
+ includeSearch,
+ allowRearrangeColumns,
+ pagination,
+ pageSize,
+ }) => {
+ const gridRef = useRef<AgGridReact>(null);
+ const gridApiRef = useRef<GridApi | null>(null);
+
+ const defaultColDef = useMemo<ColDef>(
+ () => ({
+ flex: 1,
+ filter: true,
+ enableRowGroup: true,
+ enableValue: true,
+ sortable: true,
+ resizable: true,
+ minWidth: 100,
+ }),
+ [],
+ );
+
+ // Handle grid ready event
+ const handleGridReady = useCallback(
+ (params: GridReadyEvent) => {
+ gridApiRef.current = params.api;
+ if (onGridReady) {
+ onGridReady(params);
+ }
+
+ // Optional: Auto-size columns on initial load
+ params.api.sizeColumnsToFit();
+ },
+ [onGridReady],
+ );
+
+ // Memoize container style
+ const containerStyle = useMemo(
+ () => ({
+ height: gridHeight
+ ? includeSearch
+ ? `${gridHeight - 16}px`
+ : `${gridHeight}px`
+ : '100%',
+ width: '100%',
+ }),
+ [gridHeight],
+ );
+
+ // Memoize grid class name
+ const gridClassName = useMemo(
+ () => `ag-theme-quartz${isDarkMode ? '-dark' : ''}`,
+ [isDarkMode],
+ );
+
+ const [quickFilterText, setQuickFilterText] = useState<string>();
+ const onFilterTextBoxChanged = useCallback(
+ ({ target: { value } }: ChangeEvent<HTMLInputElement>) =>
+ setQuickFilterText(value),
+ [],
+ );
+
+ console.log(pageSize, 'page size');
+
+ return (
+ <StyledContainer>
+ <div className={gridClassName} style={containerStyle}>
+ {includeSearch && (
+ <div className="input-wrapper">
+ <div className="input-container">
+ <svg
+ width="16"
+ height="16"
+ viewBox="0 0 16 16"
+ fill="none"
+ xmlns="http://www.w3.org/2000/svg"
+ >
+ <path
+ fillRule="evenodd"
+ clipRule="evenodd"
+ d="M11.5014 7.00039C11.5014 7.59133 11.385 8.1765 11.1588 8.72246C10.9327 9.26843 10.6012 9.7645 10.1833 10.1824C9.76548 10.6002 9.2694 10.9317 8.72344 11.1578C8.17747 11.384 7.59231 11.5004 7.00136 11.5004C6.41041 11.5004 5.82525 11.384 5.27929 11.1578C4.73332 10.9317 4.23725 10.6002 3.81938 10.1824C3.40152 9.7645 3.07005 9.26843 2.8439 8.72246C2.61776 8.1765 2.50136 7.59133 2.50136 7.00039C2.50136 5.80691 2.97547 4.66232 3.81938 3.81841C4.6633 2.97449 5.80789 2.50039 7.00136 2.50039C8.19484 2.50039 9.33943 2.97449 10.1833 3.81841C11.0273 4.66232 11.5014 5.80691 11.5014 7.00039ZM10.6814 11.7404C9.47574 12.6764 7.95873 13.1177 6.43916 12.9745C4.91959 12.8314 3.51171 12.1145 2.50211 10.9698C1.49252 9.8251 0.957113 8.33868 1.0049 6.81314C1.05268 5.28759 1.68006 3.83759 2.75932 2.75834C3.83857 1.67908 5.28856 1.0517 6.81411 1.00392C8.33966 0.956136 9.82608 1.49154 10.9708 2.50114C12.1154 3.51073 12.8323 4.91862 12.9755 6.43819C13.1187 7.95775 12.6773 9.47476 11.7414
10.6804L14.5314 13.4704C14.605 13.539 14.6642 13.6218 14.7051 13.7138C14.7461 13.8058 14.7682 13.9052 14.77 14.0059C14.7717 14.1066 14.7532 14.2066 14.7155 14.3C14.6778 14.3934 14.6216 14.4782 14.5504 14.5494C14.4792 14.6206 14.3943 14.6768 14.301 14.7145C14.2076 14.7522 14.1075 14.7708 14.0068 14.769C13.9061 14.7672 13.8068 14.7452 13.7148 14.7042C13.6228 14.6632 13.54 14.6041 13.4714 14.5304L10.6814 11.7404Z"
+ fill="currentColor"
+ />
+ </svg>
Review Comment:
Please make a separate Icon component for this svg part.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2166601486
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomPopover.tsx:
##########
@@ -0,0 +1,113 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { useEffect, useRef, useState, cloneElement } from 'react';
+import { styled } from '@superset-ui/core';
+
+const PopoverWrapper = styled.div`
+ position: relative;
+ display: inline-block;
+`;
+
+const PopoverContainer = styled.div`
+ ${({ theme }) => `
+ position: fixed;
+ background: ${theme.colors.grayscale.light4};
+ border: 1px solid ${theme.colors.grayscale.light2};
+ border-radius: ${theme.borderRadius}px;
+ box-shadow: 0 ${theme.sizeUnit / 2}px ${theme.sizeUnit * 2}px ${theme.colors.grayscale.light1}40;
Review Comment:
Initially i did try to use the ant popover ... But the ant popover renders directly on the document.body where the Ag Grid Styles gets stripped and it begins to appear in plain html
I did try to provide the anchor element to Ant Popover but it would still have issues hence i resorted to creating a popover
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2167100496
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomHeader.tsx:
##########
@@ -0,0 +1,262 @@
+/* eslint-disable jsx-a11y/no-static-element-interactions */
+/* eslint-disable theme-colors/no-literal-colors */
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { useRef, useState } from 'react';
+import { styled, t } from '@superset-ui/core';
+import { ArrowDownOutlined, ArrowUpOutlined } from '@ant-design/icons';
+import CustomPopover from './CustomPopover';
+import FilterIcon from './Filter';
+import KebabMenu from './KebabMenu';
+import {
+ CustomColDef,
+ CustomHeaderParams,
+ SortState,
+ UserProvidedColDef,
+} from '../../types';
+
+// Styled Components
+const Container = styled.div`
+ ${({ theme }) => `
+ display: flex;
+ width: 100%;
+
+ .three-dots-menu {
+ align-self: center;
+ margin-left: ${theme.sizeUnit}px;
+ cursor: pointer;
+ padding: ${theme.sizeUnit / 2}px;
+ border-radius: ${theme.borderRadius}px;
+ }
+ `}
+`;
+
+const HeaderContainer = styled.div`
+ ${({ theme }) => `
+ width: 100%;
+ display: flex;
+ align-items: center;
+ cursor: pointer;
+ padding: 0 ${theme.sizeUnit * 2}px;
+ overflow: hidden;
+ `}
+`;
+
+const HeaderLabel = styled.span`
+ ${({ theme }) => `
+ font-weight: ${theme.fontWeightStrong};
+ white-space: nowrap;
+ overflow: hidden;
+ text-overflow: ellipsis;
+ display: block;
+ max-width: 100%;
+ `}
+`;
+
+const SortIconWrapper = styled.div`
+ ${({ theme }) => `
+ display: flex;
+ align-items: center;
+ margin-left: ${theme.sizeUnit * 2}px;
+ `}
+`;
+
+const FilterIconWrapper = styled.div`
+ align-self: flex-end;
+ margin-left: auto;
+ cursor: pointer;
+`;
+
+const MenuContainer = styled.div`
+ ${({ theme }) => `
+ min-width: ${theme.sizeUnit * 45}px;
+ padding: ${theme.sizeUnit}px 0;
+
+ .menu-item {
+ padding: ${theme.sizeUnit * 2}px ${theme.sizeUnit * 4}px;
+ cursor: pointer;
+ display: flex;
+ align-items: center;
+ gap: ${theme.sizeUnit * 2}px;
+
+ &:hover {
+ background-color: ${theme.colors.primary.light4};
+ }
+ }
+
+ .menu-divider {
+ height: 1px;
+ background-color: ${theme.colors.grayscale.light2};
+ margin: ${theme.sizeUnit}px 0;
+ }
+ `}
+`;
+
+const getSortIcon = (
+ sortState: SortState[],
+ colId: string | null,
+): React.ReactNode => {
+ if (!sortState?.length || !colId) return null;
+ const currentSort = sortState[0];
+ if (currentSort.colId === colId) {
+ if (currentSort.sort === 'asc') return <ArrowUpOutlined />;
+ if (currentSort.sort === 'desc') return <ArrowDownOutlined />;
+ }
+ return null;
+};
+
+const CustomHeader: React.FC<CustomHeaderParams> = ({
+ displayName,
+ enableSorting,
+ setSort,
+ context,
+ column,
+ api,
+}) => {
+ const { initialSortState, onColumnHeaderClicked } = context;
+ const colId = column?.getColId();
+ const isPercentMetric = (column?.getColDef() as CustomColDef)?.context
+ ?.isPercentMetric;
+ const [isPopoverVisible, setPopoverVisible] = useState(false);
+ const filterContainerRef = useRef<HTMLDivElement>(null);
+ const [isMenuVisible, setIsMenuVisible] = useState(false);
+ const currentSort = initialSortState?.[0];
+
+ const userColumn = column.getUserProvidedColDef() as UserProvidedColDef;
+ const isMain = userColumn?.isMain;
+ const timeComparisonKey = userColumn?.timeComparisonKey || '';
+ const sortKey = isMain ? colId.replace('Main', '').trim() : colId;
+ const isTimeComparison = !isMain && timeComparisonKey;
+
+ const clearSort = () => {
+ onColumnHeaderClicked({ column: { colId: sortKey, sort: null } });
+ setSort(null, false);
+ };
+
+ const handleSortAsc = () => {
+ onColumnHeaderClicked({ column: { colId: sortKey, sort: 'asc' } });
+ setSort('asc', false);
+ };
+
+ const handleSortDesc = () => {
+ onColumnHeaderClicked({ column: { colId: sortKey, sort: 'desc' } });
+ setSort('desc', false);
+ };
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2966953301
> @amaannawab923 I'm not seeing any code that was moved to core to avoid duplicating code like we mentioned [here](https://github.com/apache/superset/pull/33517#discussion_r2103032376). Am I missing anything?
@michael-s-molina Would be taking up the whole table migration in stages , So this PR will focus on complete table Parity
And the follow up PR will take up Code deduplication for sqllab & Table V2 (Ag grid table) as the Testing efforts required are too much for complete parity with Superset table
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2144984656
##########
superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts:
##########
@@ -58,6 +58,7 @@ export enum FeatureFlag {
SlackEnableAvatars = 'SLACK_ENABLE_AVATARS',
EnableDashboardScreenshotEndpoints = 'ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS',
EnableDashboardDownloadWebDriverScreenshot = 'ENABLE_DASHBOARD_DOWNLOAD_WEBDRIVER_SCREENSHOT',
+ TableV2TimeComparisonEnabled = 'TABLE_V2_TIME_COMPARISON_ENABLED',
Review Comment:
You mean the whole viz type itself ? We can do that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2145535617
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomHeader.tsx:
##########
@@ -0,0 +1,346 @@
+/* eslint-disable jsx-a11y/no-static-element-interactions */
+/* eslint-disable theme-colors/no-literal-colors */
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { useRef, useState } from 'react';
+import { styled } from '@superset-ui/core';
+import {
+ ArrowDownOutlined,
+ ArrowUpOutlined,
+ PlusOutlined,
+} from '@ant-design/icons';
+import { IHeaderParams, Column, ColDef } from 'ag-grid-community';
+import CustomPopover from './CustomPopover';
+import { CustomColDef } from '..';
+
+const ThreeDots = ({ size = 14, color = 'black' }) => (
+ <svg
+ width={size}
+ height={size}
+ viewBox="0 0 16 16"
+ fill={color}
+ xmlns="http://www.w3.org/2000/svg"
+ >
+ <circle cx="8" cy="3" r="1.2" />
+ <circle cx="8" cy="8" r="1.2" />
+ <circle cx="8" cy="13" r="1.2" />
+ </svg>
+);
+const FilterIcon = (
+ <svg width="16" height="16" viewBox="0 0 24 24" fill="currentColor">
+ <rect x="3" y="6" width="18" height="2" rx="1" />
+ <rect x="6" y="11" width="12" height="2" rx="1" />
+ <rect x="9" y="16" width="6" height="2" rx="1" />
+ </svg>
+);
+
+// Styled Components
+const Container = styled.div`
+ display: flex;
+ width: 100%;
+
+ .three-dots-menu {
+ align-self: center;
+ margin-left: 5px;
+ cursor: pointer;
+ padding: 2px;
+ border-radius: 4px;
+ }
+`;
+
+const HeaderContainer = styled.div`
+ width: 100%;
+ display: flex;
+ align-items: center;
+ cursor: pointer;
+ padding: 0 8px;
+ overflow: hidden;
+`;
+
+const HeaderLabel = styled.span`
+ font-weight: 500;
+ white-space: nowrap;
+ overflow: hidden;
+ text-overflow: ellipsis;
+ display: block;
+ max-width: 100%;
+`;
+
+const SortIconWrapper = styled.div`
+ display: flex;
+ align-items: center;
+ margin-left: 0.5rem;
+`;
+
+const FilterIconWrapper = styled.div`
+ align-self: flex-end;
+ margin-left: auto;
+ cursor: pointer;
+`;
+
+const MenuContainer = styled.div`
+ min-width: 180px;
+ padding: 4px 0;
+
+ .menu-item {
+ padding: 8px 16px;
+ cursor: pointer;
+ display: flex;
+ align-items: center;
+ gap: 8px;
+
+ &:hover {
+ background-color: rgba(32, 167, 201, 0.2);
+ }
+ }
+
+ .menu-divider {
+ height: 1px;
+ background-color: #e8e8e8;
+ margin: 4px 0;
+ }
+`;
+
+const ToggleButton = styled.div`
+ display: flex;
+ align-items: center;
+ cursor: pointer;
+ padding: 2px;
+ margin-left: 4px;
+ transition: transform 0.2s;
+
+ &:hover {
+ background: rgba(0, 0, 0, 0.04);
+ border-radius: 4px;
+ }
+`;
+
+// Export the interfaces
+export interface SortState {
+ colId: string;
+ sort: 'asc' | 'desc' | null;
+}
+
+export interface CustomContext {
+ initialSortState: SortState[];
+ onColumnHeaderClicked: (args: { column: SortState }) => void;
+}
+
+export interface CustomHeaderParams extends IHeaderParams {
+ context: CustomContext;
+ column: Column;
+}
+
+interface UserProvidedColDef extends ColDef {
+ isMain?: boolean;
+ timeComparisonKey?: string;
+}
+
+const getSortIcon = (
+ sortState: SortState[],
+ colId: string | null,
+): React.ReactNode => {
+ if (!sortState?.length || !colId) return null;
+ const currentSort = sortState[0];
+ if (currentSort.colId === colId) {
+ if (currentSort.sort === 'asc') return <ArrowUpOutlined />;
+ if (currentSort.sort === 'desc') return <ArrowDownOutlined />;
+ }
+ return null;
+};
+
+const CustomHeader: React.FC<CustomHeaderParams> = ({
+ displayName,
+ enableSorting,
+ setSort,
+ context,
+ column,
+ api,
+}) => {
+ const { initialSortState, onColumnHeaderClicked } = context;
+ const colId = column?.getColId();
+ const isPercentMetric = (column?.getColDef() as CustomColDef)?.customMeta
+ ?.isPercentMetric;
+ const [isPopoverVisible, setPopoverVisible] = useState(false);
+ const filterContainerRef = useRef<HTMLDivElement>(null);
+ const [isMenuVisible, setIsMenuVisible] = useState(false);
+ const currentSort = initialSortState?.[0];
+ const [areComparisonColumnsVisible, setAreComparisonColumnsVisible] =
+ useState(true);
+
+ const userColumn = column.getUserProvidedColDef() as UserProvidedColDef;
+ const isMain = userColumn?.isMain;
+ const timeComparisonKey = userColumn?.timeComparisonKey || '';
+ const sortKey = isMain ? colId.replace('Main', '').trim() : colId;
+ const isTimeComparison = !isMain && timeComparisonKey;
+
+ const ClearSort = () => {
+ onColumnHeaderClicked({ column: { colId: sortKey, sort: null } });
+ setSort(null, false);
+ };
+
+ const handleSortAsc = () => {
+ onColumnHeaderClicked({ column: { colId: sortKey, sort: 'asc' } });
+ setSort('asc', false);
+ };
+
+ const handleSortDesc = () => {
+ onColumnHeaderClicked({ column: { colId: sortKey, sort: 'desc' } });
+ setSort('desc', false);
+ };
+
+ const handleSort = () => {
+ if (isTimeComparison) return;
+
+ if (!enableSorting) return;
+
+ const current = initialSortState?.[0];
+
+ if (!current || current.colId !== colId) {
+ handleSortAsc();
+ } else if (current.sort === 'asc') {
+ handleSortDesc();
+ } else {
+ ClearSort();
+ }
+ };
+
+ const handleFilterClick = async (event: React.MouseEvent) => {
+ event.stopPropagation();
+ setPopoverVisible(!isPopoverVisible);
+
+ const filterInstance = await api.getColumnFilterInstance<any>(column);
+ const filterEl = filterInstance?.eGui;
+
+ if (!filterEl || !filterContainerRef.current) return;
+
+ filterContainerRef.current.innerHTML = '';
+ filterContainerRef.current.appendChild(filterEl);
+ };
+
+ const handleMenuClick = (event: React.MouseEvent) => {
+ event.stopPropagation();
+ setIsMenuVisible(!isMenuVisible);
+ };
+
+ const handleToggleComparison = (event: React.MouseEvent) => {
+ event.stopPropagation();
+
+ const allColumns = api.getColumnDefs();
+ const timeComparisonColumns = allColumns?.filter(
+ col =>
+ (col as UserProvidedColDef).timeComparisonKey === timeComparisonKey &&
+ !(col as UserProvidedColDef).isMain,
+ );
+ const timeComparsionColIds = timeComparisonColumns?.map(
+ item => (item as UserProvidedColDef).field || '',
+ ) as string[];
+ api.setColumnsVisible(timeComparsionColIds, !areComparisonColumnsVisible);
+
+ api.sizeColumnsToFit();
+
+ setAreComparisonColumnsVisible(!areComparisonColumnsVisible);
+ };
+
+ const shouldShowAsc =
+ !currentSort ||
+ (currentSort?.colId === colId && currentSort?.sort === 'desc') ||
+ currentSort?.colId !== colId;
+ const shouldShowDesc =
+ !currentSort ||
+ (currentSort?.colId === colId && currentSort?.sort === 'asc') ||
+ currentSort?.colId !== colId;
+
+ const menuContent = (
+ <MenuContainer>
+ {shouldShowAsc && !isTimeComparison && (
+ <div onClick={handleSortAsc} className="menu-item">
+ <ArrowUpOutlined /> Sort Ascending
+ </div>
+ )}
+ {shouldShowDesc && !isTimeComparison && (
+ <div onClick={handleSortDesc} className="menu-item">
+ <ArrowDownOutlined /> Sort Descending
+ </div>
+ )}
+ {currentSort && currentSort?.colId === colId && (
+ <div onClick={ClearSort} className="menu-item">
+ <span style={{ fontSize: 16 }}>↻</span> Clear Sort
+ </div>
+ )}
+ </MenuContainer>
+ );
+
+ return (
+ <Container>
+ <HeaderContainer onClick={handleSort} className="custom-header">
+ <HeaderLabel>{displayName}</HeaderLabel>
+ <SortIconWrapper>
+ {getSortIcon(initialSortState, colId)}
+ </SortIconWrapper>
+ {isMain && timeComparisonKey && (
+ <ToggleButton
+ onClick={handleToggleComparison}
+ title={
+ areComparisonColumnsVisible
+ ? 'Hide comparison columns'
Review Comment:
Should these be localized / translatable?
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomHeader.tsx:
##########
@@ -0,0 +1,346 @@
+/* eslint-disable jsx-a11y/no-static-element-interactions */
+/* eslint-disable theme-colors/no-literal-colors */
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { useRef, useState } from 'react';
+import { styled } from '@superset-ui/core';
+import {
+ ArrowDownOutlined,
+ ArrowUpOutlined,
+ PlusOutlined,
+} from '@ant-design/icons';
+import { IHeaderParams, Column, ColDef } from 'ag-grid-community';
+import CustomPopover from './CustomPopover';
+import { CustomColDef } from '..';
+
+const ThreeDots = ({ size = 14, color = 'black' }) => (
+ <svg
+ width={size}
+ height={size}
+ viewBox="0 0 16 16"
+ fill={color}
+ xmlns="http://www.w3.org/2000/svg"
+ >
+ <circle cx="8" cy="3" r="1.2" />
+ <circle cx="8" cy="8" r="1.2" />
+ <circle cx="8" cy="13" r="1.2" />
+ </svg>
+);
+const FilterIcon = (
+ <svg width="16" height="16" viewBox="0 0 24 24" fill="currentColor">
+ <rect x="3" y="6" width="18" height="2" rx="1" />
+ <rect x="6" y="11" width="12" height="2" rx="1" />
+ <rect x="9" y="16" width="6" height="2" rx="1" />
+ </svg>
+);
+
+// Styled Components
+const Container = styled.div`
+ display: flex;
+ width: 100%;
+
+ .three-dots-menu {
+ align-self: center;
+ margin-left: 5px;
+ cursor: pointer;
+ padding: 2px;
+ border-radius: 4px;
+ }
+`;
+
+const HeaderContainer = styled.div`
+ width: 100%;
+ display: flex;
+ align-items: center;
+ cursor: pointer;
+ padding: 0 8px;
+ overflow: hidden;
+`;
+
+const HeaderLabel = styled.span`
+ font-weight: 500;
+ white-space: nowrap;
+ overflow: hidden;
+ text-overflow: ellipsis;
+ display: block;
+ max-width: 100%;
+`;
+
+const SortIconWrapper = styled.div`
+ display: flex;
+ align-items: center;
+ margin-left: 0.5rem;
+`;
+
+const FilterIconWrapper = styled.div`
+ align-self: flex-end;
+ margin-left: auto;
+ cursor: pointer;
+`;
+
+const MenuContainer = styled.div`
+ min-width: 180px;
+ padding: 4px 0;
+
+ .menu-item {
+ padding: 8px 16px;
+ cursor: pointer;
+ display: flex;
+ align-items: center;
+ gap: 8px;
+
+ &:hover {
+ background-color: rgba(32, 167, 201, 0.2);
+ }
+ }
+
+ .menu-divider {
+ height: 1px;
+ background-color: #e8e8e8;
+ margin: 4px 0;
+ }
+`;
+
+const ToggleButton = styled.div`
+ display: flex;
+ align-items: center;
+ cursor: pointer;
+ padding: 2px;
+ margin-left: 4px;
+ transition: transform 0.2s;
+
+ &:hover {
+ background: rgba(0, 0, 0, 0.04);
+ border-radius: 4px;
+ }
+`;
+
+// Export the interfaces
+export interface SortState {
+ colId: string;
+ sort: 'asc' | 'desc' | null;
+}
+
+export interface CustomContext {
+ initialSortState: SortState[];
+ onColumnHeaderClicked: (args: { column: SortState }) => void;
+}
+
+export interface CustomHeaderParams extends IHeaderParams {
+ context: CustomContext;
+ column: Column;
+}
+
+interface UserProvidedColDef extends ColDef {
+ isMain?: boolean;
+ timeComparisonKey?: string;
+}
+
+const getSortIcon = (
+ sortState: SortState[],
+ colId: string | null,
+): React.ReactNode => {
+ if (!sortState?.length || !colId) return null;
+ const currentSort = sortState[0];
+ if (currentSort.colId === colId) {
+ if (currentSort.sort === 'asc') return <ArrowUpOutlined />;
+ if (currentSort.sort === 'desc') return <ArrowDownOutlined />;
+ }
+ return null;
+};
+
+const CustomHeader: React.FC<CustomHeaderParams> = ({
+ displayName,
+ enableSorting,
+ setSort,
+ context,
+ column,
+ api,
+}) => {
+ const { initialSortState, onColumnHeaderClicked } = context;
+ const colId = column?.getColId();
+ const isPercentMetric = (column?.getColDef() as CustomColDef)?.customMeta
+ ?.isPercentMetric;
+ const [isPopoverVisible, setPopoverVisible] = useState(false);
+ const filterContainerRef = useRef<HTMLDivElement>(null);
+ const [isMenuVisible, setIsMenuVisible] = useState(false);
+ const currentSort = initialSortState?.[0];
+ const [areComparisonColumnsVisible, setAreComparisonColumnsVisible] =
+ useState(true);
+
+ const userColumn = column.getUserProvidedColDef() as UserProvidedColDef;
+ const isMain = userColumn?.isMain;
+ const timeComparisonKey = userColumn?.timeComparisonKey || '';
+ const sortKey = isMain ? colId.replace('Main', '').trim() : colId;
+ const isTimeComparison = !isMain && timeComparisonKey;
+
+ const ClearSort = () => {
+ onColumnHeaderClicked({ column: { colId: sortKey, sort: null } });
+ setSort(null, false);
+ };
+
+ const handleSortAsc = () => {
+ onColumnHeaderClicked({ column: { colId: sortKey, sort: 'asc' } });
+ setSort('asc', false);
+ };
+
+ const handleSortDesc = () => {
+ onColumnHeaderClicked({ column: { colId: sortKey, sort: 'desc' } });
+ setSort('desc', false);
+ };
+
+ const handleSort = () => {
+ if (isTimeComparison) return;
+
+ if (!enableSorting) return;
+
+ const current = initialSortState?.[0];
+
+ if (!current || current.colId !== colId) {
+ handleSortAsc();
+ } else if (current.sort === 'asc') {
+ handleSortDesc();
+ } else {
+ ClearSort();
Review Comment:
```suggestion
clearSort();
```
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/Pagination.tsx:
##########
@@ -0,0 +1,204 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/* eslint-disable theme-colors/no-literal-colors */
+import { styled } from '@superset-ui/core';
+import {
+ VerticalLeftOutlined,
+ VerticalRightOutlined,
+ LeftOutlined,
+ RightOutlined,
+} from '@ant-design/icons';
+
+const PaginationContainer = styled.div`
+ border: 1px solid #dcdddd;
+ border-bottom-left-radius: 10px;
+ border-bottom-right-radius: 10px;
+ display: flex;
+ align-items: center;
+ justify-content: flex-end;
+ padding: 8px 16px;
+ border-top: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
+ font-size: 14px;
+ color: ${({ theme }) => theme.colors.grayscale.dark1};
+ transform: translateY(-5px);
+ background: white;
+`;
+
+const SelectWrapper = styled.div`
+ position: relative;
+ display: inline-block;
+ min-width: 70px;
+`;
+
+const StyledSelect = styled.select<{ numberLength: number }>`
+ width: auto;
+ margin: 0 8px;
+ padding: 2px 24px 2px 8px;
+ border: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
+ border-radius: 4px;
+ background: white;
+ appearance: none;
+ cursor: pointer;
+
+ /* Custom arrow styling */
+ background-image: url("data:image/svg+xml;charset=UTF-8,%3csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 24 24' fill='%23000000'%3e%3cpath d='M7 10l5 5 5-5z'/%3e%3c/svg%3e");
+ background-repeat: no-repeat;
+ background-position: right
+ ${props => (props.numberLength <= 2 ? '8px' : '4px')} center;
+ background-size: 24px;
+
+ &:hover {
+ border-color: ${({ theme }) => theme.colors.grayscale.dark1};
+ }
+`;
+
+const PageInfo = styled.span`
+ margin: 0 24px;
+ span {
+ font-weight: 500;
+ }
+`;
+
+const PageCount = styled.span`
+ span {
+ font-weight: 500;
+ }
+`;
+const ButtonGroup = styled.div`
+ display: flex;
+ gap: 12px;
+`;
+
+interface PageButtonProps {
+ disabled?: boolean;
+}
+
+const PageButton = styled.div<PageButtonProps>`
+ cursor: ${({ disabled }) => (disabled ? 'not-allowed' : 'pointer')};
+ display: flex;
+ align-items: center;
+ justify-content: center;
+
+ svg {
+ height: 12px;
+ width: 12px;
+ fill: ${({ theme, disabled }) =>
+ disabled ? theme.colors.grayscale.light1 : theme.colors.grayscale.dark2};
+ }
+`;
+
+interface PaginationProps {
+ currentPage: number;
+ pageSize: number;
+ totalRows: number;
+ pageSizeOptions: number[];
+ onServerPaginationChange: (pageNumber: number, pageSize: number) => void;
+ onServerPageSizeChange: (pageSize: number) => void;
+}
+
+const Pagination: React.FC<PaginationProps> = ({
+ currentPage = 0,
+ pageSize = 10,
+ totalRows = 0,
+ pageSizeOptions = [10, 20, 50, 100, 200],
+ onServerPaginationChange = () => {},
+ onServerPageSizeChange = () => {},
+}) => {
+ const totalPages = Math.ceil(totalRows / pageSize);
+ const startRow = currentPage * pageSize + 1;
+ const endRow = Math.min((currentPage + 1) * pageSize, totalRows);
+
+ const handleNextPage = (disabled: boolean) => () => {
+ if (disabled) return;
+ onServerPaginationChange(currentPage + 1, pageSize);
+ };
+
+ const handlePrevPage = (disabled: boolean) => () => {
+ if (disabled) return;
+ onServerPaginationChange(currentPage - 1, pageSize);
+ };
+
+ const handleNavigateToFirstPage = (disabled: boolean) => () => {
+ if (disabled) return;
+ onServerPaginationChange(0, pageSize);
+ };
+
+ const handleNavigateToLastPage = (disabled: boolean) => () => {
+ if (disabled) return;
+ onServerPaginationChange(totalPages - 1, pageSize);
+ };
+
+ return (
+ <PaginationContainer>
+ <span>Page Size:</span>
Review Comment:
Localize?
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/Pagination.tsx:
##########
@@ -0,0 +1,204 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/* eslint-disable theme-colors/no-literal-colors */
+import { styled } from '@superset-ui/core';
+import {
+ VerticalLeftOutlined,
+ VerticalRightOutlined,
+ LeftOutlined,
+ RightOutlined,
+} from '@ant-design/icons';
+
+const PaginationContainer = styled.div`
+ border: 1px solid #dcdddd;
+ border-bottom-left-radius: 10px;
+ border-bottom-right-radius: 10px;
+ display: flex;
+ align-items: center;
+ justify-content: flex-end;
+ padding: 8px 16px;
+ border-top: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
+ font-size: 14px;
+ color: ${({ theme }) => theme.colors.grayscale.dark1};
+ transform: translateY(-5px);
+ background: white;
+`;
+
+const SelectWrapper = styled.div`
+ position: relative;
+ display: inline-block;
+ min-width: 70px;
+`;
+
+const StyledSelect = styled.select<{ numberLength: number }>`
+ width: auto;
+ margin: 0 8px;
+ padding: 2px 24px 2px 8px;
+ border: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
+ border-radius: 4px;
+ background: white;
+ appearance: none;
+ cursor: pointer;
+
+ /* Custom arrow styling */
+ background-image: url("data:image/svg+xml;charset=UTF-8,%3csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 24 24' fill='%23000000'%3e%3cpath d='M7 10l5 5 5-5z'/%3e%3c/svg%3e");
+ background-repeat: no-repeat;
+ background-position: right
+ ${props => (props.numberLength <= 2 ? '8px' : '4px')} center;
+ background-size: 24px;
+
+ &:hover {
+ border-color: ${({ theme }) => theme.colors.grayscale.dark1};
+ }
+`;
+
+const PageInfo = styled.span`
+ margin: 0 24px;
+ span {
+ font-weight: 500;
+ }
+`;
+
+const PageCount = styled.span`
+ span {
+ font-weight: 500;
+ }
+`;
+const ButtonGroup = styled.div`
+ display: flex;
+ gap: 12px;
+`;
+
+interface PageButtonProps {
+ disabled?: boolean;
+}
+
+const PageButton = styled.div<PageButtonProps>`
+ cursor: ${({ disabled }) => (disabled ? 'not-allowed' : 'pointer')};
+ display: flex;
+ align-items: center;
+ justify-content: center;
+
+ svg {
+ height: 12px;
+ width: 12px;
+ fill: ${({ theme, disabled }) =>
+ disabled ? theme.colors.grayscale.light1 : theme.colors.grayscale.dark2};
+ }
+`;
+
+interface PaginationProps {
+ currentPage: number;
+ pageSize: number;
+ totalRows: number;
+ pageSizeOptions: number[];
+ onServerPaginationChange: (pageNumber: number, pageSize: number) => void;
+ onServerPageSizeChange: (pageSize: number) => void;
+}
+
+const Pagination: React.FC<PaginationProps> = ({
+ currentPage = 0,
+ pageSize = 10,
+ totalRows = 0,
+ pageSizeOptions = [10, 20, 50, 100, 200],
+ onServerPaginationChange = () => {},
+ onServerPageSizeChange = () => {},
+}) => {
+ const totalPages = Math.ceil(totalRows / pageSize);
+ const startRow = currentPage * pageSize + 1;
+ const endRow = Math.min((currentPage + 1) * pageSize, totalRows);
+
+ const handleNextPage = (disabled: boolean) => () => {
+ if (disabled) return;
+ onServerPaginationChange(currentPage + 1, pageSize);
+ };
+
+ const handlePrevPage = (disabled: boolean) => () => {
+ if (disabled) return;
+ onServerPaginationChange(currentPage - 1, pageSize);
+ };
+
+ const handleNavigateToFirstPage = (disabled: boolean) => () => {
+ if (disabled) return;
+ onServerPaginationChange(0, pageSize);
+ };
+
+ const handleNavigateToLastPage = (disabled: boolean) => () => {
+ if (disabled) return;
+ onServerPaginationChange(totalPages - 1, pageSize);
+ };
+
+ return (
+ <PaginationContainer>
+ <span>Page Size:</span>
+ <SelectWrapper>
+ <StyledSelect
+ numberLength={pageSize.toString().length}
+ onChange={e => {
+ onServerPageSizeChange(Number(e.target.value));
+ }}
+ value={pageSize}
+ >
+ {pageSizeOptions.map(size => (
+ <option key={size} value={size}>
+ {size}
+ </option>
+ ))}
+ </StyledSelect>
+ </SelectWrapper>
+
+ <PageInfo>
+ <span>{startRow}</span> to <span>{endRow}</span> of{' '}
Review Comment:
Should we localize these and other strings?
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomHeader.tsx:
##########
@@ -0,0 +1,346 @@
+/* eslint-disable jsx-a11y/no-static-element-interactions */
+/* eslint-disable theme-colors/no-literal-colors */
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { useRef, useState } from 'react';
+import { styled } from '@superset-ui/core';
+import {
+ ArrowDownOutlined,
+ ArrowUpOutlined,
+ PlusOutlined,
+} from '@ant-design/icons';
+import { IHeaderParams, Column, ColDef } from 'ag-grid-community';
+import CustomPopover from './CustomPopover';
+import { CustomColDef } from '..';
+
+const ThreeDots = ({ size = 14, color = 'black' }) => (
+ <svg
+ width={size}
+ height={size}
+ viewBox="0 0 16 16"
+ fill={color}
+ xmlns="http://www.w3.org/2000/svg"
+ >
+ <circle cx="8" cy="3" r="1.2" />
+ <circle cx="8" cy="8" r="1.2" />
+ <circle cx="8" cy="13" r="1.2" />
+ </svg>
+);
+const FilterIcon = (
+ <svg width="16" height="16" viewBox="0 0 24 24" fill="currentColor">
+ <rect x="3" y="6" width="18" height="2" rx="1" />
+ <rect x="6" y="11" width="12" height="2" rx="1" />
+ <rect x="9" y="16" width="6" height="2" rx="1" />
+ </svg>
+);
+
+// Styled Components
+const Container = styled.div`
+ display: flex;
+ width: 100%;
+
+ .three-dots-menu {
+ align-self: center;
+ margin-left: 5px;
+ cursor: pointer;
+ padding: 2px;
+ border-radius: 4px;
+ }
+`;
+
+const HeaderContainer = styled.div`
+ width: 100%;
+ display: flex;
+ align-items: center;
+ cursor: pointer;
+ padding: 0 8px;
+ overflow: hidden;
+`;
+
+const HeaderLabel = styled.span`
+ font-weight: 500;
+ white-space: nowrap;
+ overflow: hidden;
+ text-overflow: ellipsis;
+ display: block;
+ max-width: 100%;
+`;
+
+const SortIconWrapper = styled.div`
+ display: flex;
+ align-items: center;
+ margin-left: 0.5rem;
+`;
+
+const FilterIconWrapper = styled.div`
+ align-self: flex-end;
+ margin-left: auto;
+ cursor: pointer;
+`;
+
+const MenuContainer = styled.div`
+ min-width: 180px;
+ padding: 4px 0;
+
+ .menu-item {
+ padding: 8px 16px;
+ cursor: pointer;
+ display: flex;
+ align-items: center;
+ gap: 8px;
+
+ &:hover {
+ background-color: rgba(32, 167, 201, 0.2);
+ }
+ }
+
+ .menu-divider {
+ height: 1px;
+ background-color: #e8e8e8;
+ margin: 4px 0;
+ }
+`;
+
+const ToggleButton = styled.div`
+ display: flex;
+ align-items: center;
+ cursor: pointer;
+ padding: 2px;
+ margin-left: 4px;
+ transition: transform 0.2s;
+
+ &:hover {
+ background: rgba(0, 0, 0, 0.04);
+ border-radius: 4px;
+ }
+`;
+
+// Export the interfaces
+export interface SortState {
+ colId: string;
+ sort: 'asc' | 'desc' | null;
+}
+
+export interface CustomContext {
+ initialSortState: SortState[];
+ onColumnHeaderClicked: (args: { column: SortState }) => void;
+}
+
+export interface CustomHeaderParams extends IHeaderParams {
+ context: CustomContext;
+ column: Column;
+}
+
+interface UserProvidedColDef extends ColDef {
+ isMain?: boolean;
+ timeComparisonKey?: string;
+}
+
+const getSortIcon = (
+ sortState: SortState[],
+ colId: string | null,
+): React.ReactNode => {
+ if (!sortState?.length || !colId) return null;
+ const currentSort = sortState[0];
+ if (currentSort.colId === colId) {
+ if (currentSort.sort === 'asc') return <ArrowUpOutlined />;
+ if (currentSort.sort === 'desc') return <ArrowDownOutlined />;
+ }
+ return null;
+};
+
+const CustomHeader: React.FC<CustomHeaderParams> = ({
+ displayName,
+ enableSorting,
+ setSort,
+ context,
+ column,
+ api,
+}) => {
+ const { initialSortState, onColumnHeaderClicked } = context;
+ const colId = column?.getColId();
+ const isPercentMetric = (column?.getColDef() as CustomColDef)?.customMeta
+ ?.isPercentMetric;
+ const [isPopoverVisible, setPopoverVisible] = useState(false);
+ const filterContainerRef = useRef<HTMLDivElement>(null);
+ const [isMenuVisible, setIsMenuVisible] = useState(false);
+ const currentSort = initialSortState?.[0];
+ const [areComparisonColumnsVisible, setAreComparisonColumnsVisible] =
+ useState(true);
+
+ const userColumn = column.getUserProvidedColDef() as UserProvidedColDef;
+ const isMain = userColumn?.isMain;
+ const timeComparisonKey = userColumn?.timeComparisonKey || '';
+ const sortKey = isMain ? colId.replace('Main', '').trim() : colId;
+ const isTimeComparison = !isMain && timeComparisonKey;
+
+ const ClearSort = () => {
Review Comment:
```suggestion
const clearSort = () => {
```
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomHeader.tsx:
##########
@@ -0,0 +1,346 @@
+/* eslint-disable jsx-a11y/no-static-element-interactions */
+/* eslint-disable theme-colors/no-literal-colors */
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { useRef, useState } from 'react';
+import { styled } from '@superset-ui/core';
+import {
+ ArrowDownOutlined,
+ ArrowUpOutlined,
+ PlusOutlined,
+} from '@ant-design/icons';
+import { IHeaderParams, Column, ColDef } from 'ag-grid-community';
+import CustomPopover from './CustomPopover';
+import { CustomColDef } from '..';
+
+const ThreeDots = ({ size = 14, color = 'black' }) => (
+ <svg
+ width={size}
+ height={size}
+ viewBox="0 0 16 16"
+ fill={color}
+ xmlns="http://www.w3.org/2000/svg"
+ >
+ <circle cx="8" cy="3" r="1.2" />
+ <circle cx="8" cy="8" r="1.2" />
+ <circle cx="8" cy="13" r="1.2" />
+ </svg>
+);
+const FilterIcon = (
+ <svg width="16" height="16" viewBox="0 0 24 24" fill="currentColor">
+ <rect x="3" y="6" width="18" height="2" rx="1" />
+ <rect x="6" y="11" width="12" height="2" rx="1" />
+ <rect x="9" y="16" width="6" height="2" rx="1" />
+ </svg>
+);
+
+// Styled Components
+const Container = styled.div`
+ display: flex;
+ width: 100%;
+
+ .three-dots-menu {
+ align-self: center;
+ margin-left: 5px;
+ cursor: pointer;
+ padding: 2px;
+ border-radius: 4px;
+ }
+`;
+
+const HeaderContainer = styled.div`
+ width: 100%;
+ display: flex;
+ align-items: center;
+ cursor: pointer;
+ padding: 0 8px;
+ overflow: hidden;
+`;
+
+const HeaderLabel = styled.span`
+ font-weight: 500;
+ white-space: nowrap;
+ overflow: hidden;
+ text-overflow: ellipsis;
+ display: block;
+ max-width: 100%;
+`;
+
+const SortIconWrapper = styled.div`
+ display: flex;
+ align-items: center;
+ margin-left: 0.5rem;
+`;
+
+const FilterIconWrapper = styled.div`
+ align-self: flex-end;
+ margin-left: auto;
+ cursor: pointer;
+`;
+
+const MenuContainer = styled.div`
+ min-width: 180px;
+ padding: 4px 0;
+
+ .menu-item {
+ padding: 8px 16px;
+ cursor: pointer;
+ display: flex;
+ align-items: center;
+ gap: 8px;
+
+ &:hover {
+ background-color: rgba(32, 167, 201, 0.2);
+ }
+ }
+
+ .menu-divider {
+ height: 1px;
+ background-color: #e8e8e8;
+ margin: 4px 0;
+ }
+`;
+
+const ToggleButton = styled.div`
+ display: flex;
+ align-items: center;
+ cursor: pointer;
+ padding: 2px;
+ margin-left: 4px;
+ transition: transform 0.2s;
+
+ &:hover {
+ background: rgba(0, 0, 0, 0.04);
+ border-radius: 4px;
+ }
+`;
+
+// Export the interfaces
+export interface SortState {
+ colId: string;
+ sort: 'asc' | 'desc' | null;
+}
+
+export interface CustomContext {
+ initialSortState: SortState[];
+ onColumnHeaderClicked: (args: { column: SortState }) => void;
+}
+
+export interface CustomHeaderParams extends IHeaderParams {
+ context: CustomContext;
+ column: Column;
+}
+
+interface UserProvidedColDef extends ColDef {
+ isMain?: boolean;
+ timeComparisonKey?: string;
+}
+
+const getSortIcon = (
+ sortState: SortState[],
+ colId: string | null,
+): React.ReactNode => {
+ if (!sortState?.length || !colId) return null;
+ const currentSort = sortState[0];
+ if (currentSort.colId === colId) {
+ if (currentSort.sort === 'asc') return <ArrowUpOutlined />;
+ if (currentSort.sort === 'desc') return <ArrowDownOutlined />;
+ }
+ return null;
+};
+
+const CustomHeader: React.FC<CustomHeaderParams> = ({
+ displayName,
+ enableSorting,
+ setSort,
+ context,
+ column,
+ api,
+}) => {
+ const { initialSortState, onColumnHeaderClicked } = context;
+ const colId = column?.getColId();
+ const isPercentMetric = (column?.getColDef() as CustomColDef)?.customMeta
+ ?.isPercentMetric;
+ const [isPopoverVisible, setPopoverVisible] = useState(false);
+ const filterContainerRef = useRef<HTMLDivElement>(null);
+ const [isMenuVisible, setIsMenuVisible] = useState(false);
+ const currentSort = initialSortState?.[0];
+ const [areComparisonColumnsVisible, setAreComparisonColumnsVisible] =
+ useState(true);
+
+ const userColumn = column.getUserProvidedColDef() as UserProvidedColDef;
+ const isMain = userColumn?.isMain;
+ const timeComparisonKey = userColumn?.timeComparisonKey || '';
+ const sortKey = isMain ? colId.replace('Main', '').trim() : colId;
+ const isTimeComparison = !isMain && timeComparisonKey;
+
+ const ClearSort = () => {
+ onColumnHeaderClicked({ column: { colId: sortKey, sort: null } });
+ setSort(null, false);
+ };
+
+ const handleSortAsc = () => {
+ onColumnHeaderClicked({ column: { colId: sortKey, sort: 'asc' } });
+ setSort('asc', false);
+ };
+
+ const handleSortDesc = () => {
+ onColumnHeaderClicked({ column: { colId: sortKey, sort: 'desc' } });
+ setSort('desc', false);
+ };
+
+ const handleSort = () => {
+ if (isTimeComparison) return;
+
+ if (!enableSorting) return;
+
+ const current = initialSortState?.[0];
+
+ if (!current || current.colId !== colId) {
+ handleSortAsc();
+ } else if (current.sort === 'asc') {
+ handleSortDesc();
+ } else {
+ ClearSort();
+ }
+ };
+
+ const handleFilterClick = async (event: React.MouseEvent) => {
+ event.stopPropagation();
+ setPopoverVisible(!isPopoverVisible);
+
+ const filterInstance = await api.getColumnFilterInstance<any>(column);
+ const filterEl = filterInstance?.eGui;
+
+ if (!filterEl || !filterContainerRef.current) return;
+
+ filterContainerRef.current.innerHTML = '';
+ filterContainerRef.current.appendChild(filterEl);
+ };
+
+ const handleMenuClick = (event: React.MouseEvent) => {
+ event.stopPropagation();
+ setIsMenuVisible(!isMenuVisible);
+ };
+
+ const handleToggleComparison = (event: React.MouseEvent) => {
+ event.stopPropagation();
+
+ const allColumns = api.getColumnDefs();
+ const timeComparisonColumns = allColumns?.filter(
+ col =>
+ (col as UserProvidedColDef).timeComparisonKey === timeComparisonKey &&
+ !(col as UserProvidedColDef).isMain,
+ );
+ const timeComparsionColIds = timeComparisonColumns?.map(
+ item => (item as UserProvidedColDef).field || '',
+ ) as string[];
+ api.setColumnsVisible(timeComparsionColIds, !areComparisonColumnsVisible);
+
+ api.sizeColumnsToFit();
+
+ setAreComparisonColumnsVisible(!areComparisonColumnsVisible);
+ };
+
+ const shouldShowAsc =
+ !currentSort ||
+ (currentSort?.colId === colId && currentSort?.sort === 'desc') ||
+ currentSort?.colId !== colId;
+ const shouldShowDesc =
+ !currentSort ||
+ (currentSort?.colId === colId && currentSort?.sort === 'asc') ||
+ currentSort?.colId !== colId;
+
+ const menuContent = (
+ <MenuContainer>
+ {shouldShowAsc && !isTimeComparison && (
+ <div onClick={handleSortAsc} className="menu-item">
+ <ArrowUpOutlined /> Sort Ascending
+ </div>
+ )}
+ {shouldShowDesc && !isTimeComparison && (
+ <div onClick={handleSortDesc} className="menu-item">
+ <ArrowDownOutlined /> Sort Descending
+ </div>
+ )}
+ {currentSort && currentSort?.colId === colId && (
+ <div onClick={ClearSort} className="menu-item">
Review Comment:
```suggestion
<div onClick={clearSort} className="menu-item">
```
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomPopover.tsx:
##########
@@ -0,0 +1,112 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/* eslint-disable theme-colors/no-literal-colors */
+import { useEffect, useRef, useState, cloneElement } from 'react';
+import { styled } from '@superset-ui/core';
+
+const PopoverWrapper = styled.div`
+ position: relative;
+ display: inline-block;
+`;
+
+const PopoverContainer = styled.div`
+ position: fixed;
+ background: #f8f8f8;
Review Comment:
I see we are not using the theme anywhere, is that intended?
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/Pagination.tsx:
##########
@@ -0,0 +1,204 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/* eslint-disable theme-colors/no-literal-colors */
+import { styled } from '@superset-ui/core';
+import {
+ VerticalLeftOutlined,
+ VerticalRightOutlined,
+ LeftOutlined,
+ RightOutlined,
+} from '@ant-design/icons';
+
+const PaginationContainer = styled.div`
+ border: 1px solid #dcdddd;
+ border-bottom-left-radius: 10px;
+ border-bottom-right-radius: 10px;
+ display: flex;
+ align-items: center;
+ justify-content: flex-end;
+ padding: 8px 16px;
+ border-top: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
Review Comment:
Nit: We can promote the theme at the top of the styled declaration
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/Pagination.tsx:
##########
@@ -0,0 +1,204 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/* eslint-disable theme-colors/no-literal-colors */
+import { styled } from '@superset-ui/core';
+import {
+ VerticalLeftOutlined,
+ VerticalRightOutlined,
+ LeftOutlined,
+ RightOutlined,
+} from '@ant-design/icons';
+
+const PaginationContainer = styled.div`
+ border: 1px solid #dcdddd;
+ border-bottom-left-radius: 10px;
+ border-bottom-right-radius: 10px;
+ display: flex;
+ align-items: center;
+ justify-content: flex-end;
+ padding: 8px 16px;
+ border-top: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
+ font-size: 14px;
Review Comment:
Let's use the theme for font sizes, colors, paddings / margings, etc.
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomPopover.tsx:
##########
@@ -0,0 +1,112 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/* eslint-disable theme-colors/no-literal-colors */
Review Comment:
Do we need this? Theming is just around the corner so we should try to use colors from the theme I guess
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2966925311
@amaannawab923 I'm not seeing any code that was moved to core to avoid duplicating code like we mentioned [here](https://github.com/apache/superset/pull/33517#discussion_r2103032376). Am I missing anything?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2149241887
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomHeader.tsx:
##########
@@ -0,0 +1,346 @@
+/* eslint-disable jsx-a11y/no-static-element-interactions */
+/* eslint-disable theme-colors/no-literal-colors */
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { useRef, useState } from 'react';
+import { styled } from '@superset-ui/core';
+import {
+ ArrowDownOutlined,
+ ArrowUpOutlined,
+ PlusOutlined,
+} from '@ant-design/icons';
+import { IHeaderParams, Column, ColDef } from 'ag-grid-community';
+import CustomPopover from './CustomPopover';
+import { CustomColDef } from '..';
+
+const ThreeDots = ({ size = 14, color = 'black' }) => (
+ <svg
+ width={size}
+ height={size}
+ viewBox="0 0 16 16"
+ fill={color}
+ xmlns="http://www.w3.org/2000/svg"
+ >
+ <circle cx="8" cy="3" r="1.2" />
+ <circle cx="8" cy="8" r="1.2" />
+ <circle cx="8" cy="13" r="1.2" />
+ </svg>
+);
+const FilterIcon = (
+ <svg width="16" height="16" viewBox="0 0 24 24" fill="currentColor">
+ <rect x="3" y="6" width="18" height="2" rx="1" />
+ <rect x="6" y="11" width="12" height="2" rx="1" />
+ <rect x="9" y="16" width="6" height="2" rx="1" />
+ </svg>
+);
+
+// Styled Components
+const Container = styled.div`
+ display: flex;
+ width: 100%;
+
+ .three-dots-menu {
+ align-self: center;
+ margin-left: 5px;
+ cursor: pointer;
+ padding: 2px;
+ border-radius: 4px;
+ }
+`;
+
+const HeaderContainer = styled.div`
+ width: 100%;
+ display: flex;
+ align-items: center;
+ cursor: pointer;
+ padding: 0 8px;
+ overflow: hidden;
+`;
+
+const HeaderLabel = styled.span`
+ font-weight: 500;
+ white-space: nowrap;
+ overflow: hidden;
+ text-overflow: ellipsis;
+ display: block;
+ max-width: 100%;
+`;
+
+const SortIconWrapper = styled.div`
+ display: flex;
+ align-items: center;
+ margin-left: 0.5rem;
+`;
+
+const FilterIconWrapper = styled.div`
+ align-self: flex-end;
+ margin-left: auto;
+ cursor: pointer;
+`;
+
+const MenuContainer = styled.div`
+ min-width: 180px;
+ padding: 4px 0;
+
+ .menu-item {
+ padding: 8px 16px;
+ cursor: pointer;
+ display: flex;
+ align-items: center;
+ gap: 8px;
+
+ &:hover {
+ background-color: rgba(32, 167, 201, 0.2);
+ }
+ }
+
+ .menu-divider {
+ height: 1px;
+ background-color: #e8e8e8;
+ margin: 4px 0;
+ }
+`;
+
+const ToggleButton = styled.div`
+ display: flex;
+ align-items: center;
+ cursor: pointer;
+ padding: 2px;
+ margin-left: 4px;
+ transition: transform 0.2s;
+
+ &:hover {
+ background: rgba(0, 0, 0, 0.04);
+ border-radius: 4px;
+ }
+`;
+
+// Export the interfaces
+export interface SortState {
+ colId: string;
+ sort: 'asc' | 'desc' | null;
+}
+
+export interface CustomContext {
+ initialSortState: SortState[];
+ onColumnHeaderClicked: (args: { column: SortState }) => void;
+}
+
+export interface CustomHeaderParams extends IHeaderParams {
+ context: CustomContext;
+ column: Column;
+}
+
+interface UserProvidedColDef extends ColDef {
+ isMain?: boolean;
+ timeComparisonKey?: string;
+}
+
+const getSortIcon = (
+ sortState: SortState[],
+ colId: string | null,
+): React.ReactNode => {
+ if (!sortState?.length || !colId) return null;
+ const currentSort = sortState[0];
+ if (currentSort.colId === colId) {
+ if (currentSort.sort === 'asc') return <ArrowUpOutlined />;
+ if (currentSort.sort === 'desc') return <ArrowDownOutlined />;
+ }
+ return null;
+};
+
+const CustomHeader: React.FC<CustomHeaderParams> = ({
+ displayName,
+ enableSorting,
+ setSort,
+ context,
+ column,
+ api,
+}) => {
+ const { initialSortState, onColumnHeaderClicked } = context;
+ const colId = column?.getColId();
+ const isPercentMetric = (column?.getColDef() as CustomColDef)?.customMeta
+ ?.isPercentMetric;
+ const [isPopoverVisible, setPopoverVisible] = useState(false);
+ const filterContainerRef = useRef<HTMLDivElement>(null);
+ const [isMenuVisible, setIsMenuVisible] = useState(false);
+ const currentSort = initialSortState?.[0];
+ const [areComparisonColumnsVisible, setAreComparisonColumnsVisible] =
+ useState(true);
+
+ const userColumn = column.getUserProvidedColDef() as UserProvidedColDef;
+ const isMain = userColumn?.isMain;
+ const timeComparisonKey = userColumn?.timeComparisonKey || '';
+ const sortKey = isMain ? colId.replace('Main', '').trim() : colId;
+ const isTimeComparison = !isMain && timeComparisonKey;
+
+ const ClearSort = () => {
+ onColumnHeaderClicked({ column: { colId: sortKey, sort: null } });
+ setSort(null, false);
+ };
+
+ const handleSortAsc = () => {
+ onColumnHeaderClicked({ column: { colId: sortKey, sort: 'asc' } });
+ setSort('asc', false);
+ };
+
+ const handleSortDesc = () => {
+ onColumnHeaderClicked({ column: { colId: sortKey, sort: 'desc' } });
+ setSort('desc', false);
+ };
+
+ const handleSort = () => {
+ if (isTimeComparison) return;
+
+ if (!enableSorting) return;
+
+ const current = initialSortState?.[0];
+
+ if (!current || current.colId !== colId) {
+ handleSortAsc();
+ } else if (current.sort === 'asc') {
+ handleSortDesc();
+ } else {
+ ClearSort();
+ }
+ };
+
+ const handleFilterClick = async (event: React.MouseEvent) => {
+ event.stopPropagation();
+ setPopoverVisible(!isPopoverVisible);
+
+ const filterInstance = await api.getColumnFilterInstance<any>(column);
+ const filterEl = filterInstance?.eGui;
+
+ if (!filterEl || !filterContainerRef.current) return;
+
+ filterContainerRef.current.innerHTML = '';
+ filterContainerRef.current.appendChild(filterEl);
+ };
+
+ const handleMenuClick = (event: React.MouseEvent) => {
+ event.stopPropagation();
+ setIsMenuVisible(!isMenuVisible);
+ };
+
+ const handleToggleComparison = (event: React.MouseEvent) => {
+ event.stopPropagation();
+
+ const allColumns = api.getColumnDefs();
+ const timeComparisonColumns = allColumns?.filter(
+ col =>
+ (col as UserProvidedColDef).timeComparisonKey === timeComparisonKey &&
+ !(col as UserProvidedColDef).isMain,
+ );
+ const timeComparsionColIds = timeComparisonColumns?.map(
+ item => (item as UserProvidedColDef).field || '',
+ ) as string[];
+ api.setColumnsVisible(timeComparsionColIds, !areComparisonColumnsVisible);
+
+ api.sizeColumnsToFit();
+
+ setAreComparisonColumnsVisible(!areComparisonColumnsVisible);
+ };
+
+ const shouldShowAsc =
+ !currentSort ||
+ (currentSort?.colId === colId && currentSort?.sort === 'desc') ||
+ currentSort?.colId !== colId;
+ const shouldShowDesc =
+ !currentSort ||
+ (currentSort?.colId === colId && currentSort?.sort === 'asc') ||
+ currentSort?.colId !== colId;
+
+ const menuContent = (
+ <MenuContainer>
+ {shouldShowAsc && !isTimeComparison && (
+ <div onClick={handleSortAsc} className="menu-item">
+ <ArrowUpOutlined /> Sort Ascending
+ </div>
+ )}
+ {shouldShowDesc && !isTimeComparison && (
+ <div onClick={handleSortDesc} className="menu-item">
+ <ArrowDownOutlined /> Sort Descending
+ </div>
+ )}
+ {currentSort && currentSort?.colId === colId && (
+ <div onClick={ClearSort} className="menu-item">
+ <span style={{ fontSize: 16 }}>↻</span> Clear Sort
+ </div>
+ )}
+ </MenuContainer>
+ );
+
+ return (
+ <Container>
+ <HeaderContainer onClick={handleSort} className="custom-header">
+ <HeaderLabel>{displayName}</HeaderLabel>
+ <SortIconWrapper>
+ {getSortIcon(initialSortState, colId)}
+ </SortIconWrapper>
+ {isMain && timeComparisonKey && (
+ <ToggleButton
+ onClick={handleToggleComparison}
+ title={
+ areComparisonColumnsVisible
+ ? 'Hide comparison columns'
Review Comment:
Yes done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2098368361
##########
superset-frontend/packages/superset-ui-core/src/chart/types/VizType.ts:
##########
@@ -54,6 +54,7 @@ export enum VizType {
Step = 'echarts_timeseries_step',
Sunburst = 'sunburst_v2',
Table = 'table',
+ TableAgGrid = 'table_ag_grid',
Review Comment:
@justinpark @michael-s-molina Tagging as a reminder , as the above can be considered as blockers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-3035585216
@geido Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/16072180169). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2900904062
@geido Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/15185601646). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2902441031
@eschutho Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/15195937989). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2902464420
@eschutho Ephemeral environment spinning up at http://34.210.58.80:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2102931451
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx:
##########
@@ -0,0 +1,228 @@
+/* eslint-disable camelcase */
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ useCallback,
+ useMemo,
+ useRef,
+ memo,
+ useState,
+ ChangeEvent,
+} from 'react';
+
+import {
+ AllCommunityModule,
+ ClientSideRowModelModule,
+ type ColDef,
+ ModuleRegistry,
+ GridReadyEvent,
+ GridApi,
+} from 'ag-grid-community';
+import { AgGridReact } from 'ag-grid-react';
+import './styles/ag-grid.css';
+
+import { type FunctionComponent } from 'react';
+
+import { styled, css } from '@superset-ui/core';
+
+export interface Props {
+ gridTheme?: string;
+ isDarkMode?: boolean;
+ gridHeight?: number | null;
+ updateInterval?: number;
+ data?: any[];
+ onGridReady?: (params: GridReadyEvent) => void;
+ colDefsFromProps: any[];
+ includeSearch: boolean;
+ allowRearrangeColumns: boolean;
+ pagination: boolean;
+ pageSize: number;
+}
+
+ModuleRegistry.registerModules([AllCommunityModule, ClientSideRowModelModule]);
+
+const StyledContainer = styled.div`
+ ${({ theme }) => css`
+ .input-container {
+ margin-left: auto;
+ }
+ .input-wrapper {
+ position: relative;
+ display: flex;
+ align-items: center;
+ overflow: visible;
+ margin-bottom: 1rem;
+ }
+
+ .input-wrapper svg {
+ pointer-events: none;
+ transform: translate(28px, 2px);
+ color: ${theme.colors.grayscale.base};
+ }
+
+ .input-wrapper input {
+ font-size: ${theme.typography.sizes.s}px;
+ padding: 0.375em 0.75em 0.375em 2.4em;
+ line-height: 1.4;
+ border-radius: ${theme.gridUnit}px;
+ border: 1px solid ${theme.colors.grayscale.light2};
+ background-color: transparent;
+ outline: none;
+
+ &:focus {
+ border-color: ${theme.colors.primary.base};
+ }
+
+ &::placeholder {
+ color: ${theme.colors.grayscale.light1};
+ }
+ }
+ `}
+`;
+
+// Memoize the entire component
+const AgGridDataTable: FunctionComponent<Props> = memo(
+ ({
+ gridTheme = 'ag-theme-quartz',
+ isDarkMode = false,
+ gridHeight = null,
+ data = [],
+ onGridReady,
+ colDefsFromProps,
+ includeSearch,
+ allowRearrangeColumns,
+ pagination,
+ pageSize,
+ }) => {
+ const gridRef = useRef<AgGridReact>(null);
+ const gridApiRef = useRef<GridApi | null>(null);
+
+ const defaultColDef = useMemo<ColDef>(
+ () => ({
+ flex: 1,
+ filter: true,
+ enableRowGroup: true,
+ enableValue: true,
+ sortable: true,
+ resizable: true,
+ minWidth: 100,
+ }),
+ [],
+ );
+
+ // Handle grid ready event
+ const handleGridReady = useCallback(
+ (params: GridReadyEvent) => {
+ gridApiRef.current = params.api;
+ if (onGridReady) {
+ onGridReady(params);
+ }
+
+ // Optional: Auto-size columns on initial load
+ params.api.sizeColumnsToFit();
+ },
+ [onGridReady],
+ );
+
+ // Memoize container style
+ const containerStyle = useMemo(
+ () => ({
+ height: gridHeight
+ ? includeSearch
+ ? `${gridHeight - 16}px`
+ : `${gridHeight}px`
+ : '100%',
+ width: '100%',
+ }),
+ [gridHeight],
+ );
+
+ // Memoize grid class name
+ const gridClassName = useMemo(
+ () => `ag-theme-quartz${isDarkMode ? '-dark' : ''}`,
+ [isDarkMode],
+ );
+
+ const [quickFilterText, setQuickFilterText] = useState<string>();
+ const onFilterTextBoxChanged = useCallback(
+ ({ target: { value } }: ChangeEvent<HTMLInputElement>) =>
+ setQuickFilterText(value),
+ [],
+ );
+
+ console.log(pageSize, 'page size');
+
+ return (
+ <StyledContainer>
+ <div className={gridClassName} style={containerStyle}>
+ {includeSearch && (
+ <div className="input-wrapper">
+ <div className="input-container">
+ <svg
+ width="16"
+ height="16"
+ viewBox="0 0 16 16"
+ fill="none"
+ xmlns="http://www.w3.org/2000/svg"
+ >
+ <path
+ fillRule="evenodd"
+ clipRule="evenodd"
+ d="M11.5014 7.00039C11.5014 7.59133 11.385 8.1765 11.1588 8.72246C10.9327 9.26843 10.6012 9.7645 10.1833 10.1824C9.76548 10.6002 9.2694 10.9317 8.72344 11.1578C8.17747 11.384 7.59231 11.5004 7.00136 11.5004C6.41041 11.5004 5.82525 11.384 5.27929 11.1578C4.73332 10.9317 4.23725 10.6002 3.81938 10.1824C3.40152 9.7645 3.07005 9.26843 2.8439 8.72246C2.61776 8.1765 2.50136 7.59133 2.50136 7.00039C2.50136 5.80691 2.97547 4.66232 3.81938 3.81841C4.6633 2.97449 5.80789 2.50039 7.00136 2.50039C8.19484 2.50039 9.33943 2.97449 10.1833 3.81841C11.0273 4.66232 11.5014 5.80691 11.5014 7.00039ZM10.6814 11.7404C9.47574 12.6764 7.95873 13.1177 6.43916 12.9745C4.91959 12.8314 3.51171 12.1145 2.50211 10.9698C1.49252 9.8251 0.957113 8.33868 1.0049 6.81314C1.05268 5.28759 1.68006 3.83759 2.75932 2.75834C3.83857 1.67908 5.28856 1.0517 6.81411 1.00392C8.33966 0.956136 9.82608 1.49154 10.9708 2.50114C12.1154 3.51073 12.8323 4.91862 12.9755 6.43819C13.1187 7.95775 12.6773 9.47476 11.7414
10.6804L14.5314 13.4704C14.605 13.539 14.6642 13.6218 14.7051 13.7138C14.7461 13.8058 14.7682 13.9052 14.77 14.0059C14.7717 14.1066 14.7532 14.2066 14.7155 14.3C14.6778 14.3934 14.6216 14.4782 14.5504 14.5494C14.4792 14.6206 14.3943 14.6768 14.301 14.7145C14.2076 14.7522 14.1075 14.7708 14.0068 14.769C13.9061 14.7672 13.8068 14.7452 13.7148 14.7042C13.6228 14.6632 13.54 14.6041 13.4714 14.5304L10.6814 11.7404Z"
+ fill="currentColor"
+ />
+ </svg>
+
+ <input
+ type="text"
+ id="filter-text-box"
+ placeholder="Search"
+ onInput={onFilterTextBoxChanged}
+ />
+ </div>
+ </div>
+ )}
+
+ <AgGridReact
Review Comment:
Any component that is used in viz plugin must be part of @superset-ui/core ... if we try to use a Component from src/components in viz plugin , we start getting errors , Should we move the Grid Table to superset-ui/core
I am unable to use src/components/GridTable in plugins directory ,
Also none of the viz plugins use src/components
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2967024732
> @michael-s-molina Would be taking up the whole table migration in stages , So this PR will focus on complete table Parity
And the follow up PR will take up Code deduplication for sqllab & Table V2 (Ag grid table) as the Testing efforts required are too much for complete parity with Superset table
@amaannawab923 @geido Are we fully committed with these migrations? The worst thing that can happen is supporting another table in the codebase. This is reminding me of the multiple select versions (7) we had in the past and the huge effort we did to unify them. Currently, with this PR we'll have 4 different table implementations:
```
- plugin-chart-table
- plugin-chart-ag-grid-table
- src/component/GridTable
- src/components/Table
```
cc @justinpark @villebro @mistercrunch @rusackas
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2967133807
> > Migration should be pretty easy going forward
>
> @amaannawab923 Just to be clear, there are multiple migrations:
>
> 1. Migrating `plugin-chart-table` to `plugin-chart-ag-grid-table`
> 2. Extracting common parts of `plugin-chart-ag-grid-table` and `GridTable` (used in SQL Lab) into a component in core to avoid code duplication
> 3. Replace `src/components/Table` with the core implementation
>
> Ideally, we would have a single table component inside core that could be extended to deal with specific things like extra plugin configurations, SQL Lab specific things, etc.
Agreed , i will try my best to wrap up all the migrations in the follow up pr
Thanks for listing it down , Will make a note of it & try my best to take care of all of the points
Thanks @michael-s-molina
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2146298113
##########
superset-frontend/packages/superset-ui-core/src/chart/types/VizType.ts:
##########
@@ -54,6 +54,7 @@ export enum VizType {
Step = 'echarts_timeseries_step',
Sunburst = 'sunburst_v2',
Table = 'table',
+ TableAgGrid = 'table_ag_grid',
Review Comment:
I agree with naming this `table_v2` also.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2164603858
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/package.json:
##########
@@ -27,6 +27,8 @@
"@react-icons/all-files": "^4.1.0",
"@types/d3-array": "^2.9.0",
"@types/react-table": "^7.7.20",
+ "ag-grid-community": "^33.1.1",
Review Comment:
should this be in `peerDependencies`? Also wondering if we need to rethink where the main deps should live and how the peerDeps should be set. I think what makes more sense is to add the main dep in `@superset-ui/core` and have the main package/app AND plugins set them as peerDep.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2966643801
@geido Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/15611360435). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2166569285
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomPopover.tsx:
##########
@@ -0,0 +1,113 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { useEffect, useRef, useState, cloneElement } from 'react';
+import { styled } from '@superset-ui/core';
+
+const PopoverWrapper = styled.div`
+ position: relative;
+ display: inline-block;
+`;
+
+const PopoverContainer = styled.div`
+ ${({ theme }) => `
+ position: fixed;
+ background: ${theme.colors.grayscale.light4};
+ border: 1px solid ${theme.colors.grayscale.light2};
+ border-radius: ${theme.borderRadius}px;
+ box-shadow: 0 ${theme.sizeUnit / 2}px ${theme.sizeUnit * 2}px ${theme.colors.grayscale.light1}40;
Review Comment:
What does this component do that requires a custom implementation? Can we use an Ant Design vanilla component for this so that we can avoid style customizations as well?
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/SearchSelectDropdown.tsx:
##########
@@ -0,0 +1,55 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/* eslint-disable import/no-extraneous-dependencies */
+import { styled } from '@superset-ui/core';
+import { Select } from 'antd';
Review Comment:
As we moved all our components to superset ui core, should we bring in the official select component instead of importing directly from antd?
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/styles/ag-grid.css:
##########
@@ -0,0 +1,6924 @@
+/* eslint-disable camelcase */
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+.ag-measurement-container {
Review Comment:
I haven't looked into this css very deeply but it makes me wonder whether themability works with this now that the theming work is part of master
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/Pagination.tsx:
##########
@@ -0,0 +1,216 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/* eslint-disable theme-colors/no-literal-colors */
+import { styled, t } from '@superset-ui/core';
+import {
+ VerticalLeftOutlined,
+ VerticalRightOutlined,
+ LeftOutlined,
+ RightOutlined,
+} from '@ant-design/icons';
+
+const PaginationContainer = styled.div`
Review Comment:
I am surprised that we need to build a custom pagination component. What is the driving decision behind this? If we really need to, should we use a standard Ant Design pagination component? https://ant.design/components/pagination
##########
superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts:
##########
@@ -60,6 +60,7 @@ export enum FeatureFlag {
SlackEnableAvatars = 'SLACK_ENABLE_AVATARS',
EnableDashboardScreenshotEndpoints = 'ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS',
EnableDashboardDownloadWebDriverScreenshot = 'ENABLE_DASHBOARD_DOWNLOAD_WEBDRIVER_SCREENSHOT',
+ TableV2TimeComparisonEnabled = 'TABLE_V2_TIME_COMPARISON_ENABLED',
Review Comment:
@amaannawab923 should feature flags also now be the same across Superset / Preset?
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomHeader.tsx:
##########
@@ -0,0 +1,262 @@
+/* eslint-disable jsx-a11y/no-static-element-interactions */
+/* eslint-disable theme-colors/no-literal-colors */
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { useRef, useState } from 'react';
+import { styled, t } from '@superset-ui/core';
+import { ArrowDownOutlined, ArrowUpOutlined } from '@ant-design/icons';
+import CustomPopover from './CustomPopover';
+import FilterIcon from './Filter';
+import KebabMenu from './KebabMenu';
+import {
+ CustomColDef,
+ CustomHeaderParams,
+ SortState,
+ UserProvidedColDef,
+} from '../../types';
+
+// Styled Components
+const Container = styled.div`
+ ${({ theme }) => `
+ display: flex;
+ width: 100%;
+
+ .three-dots-menu {
+ align-self: center;
+ margin-left: ${theme.sizeUnit}px;
+ cursor: pointer;
+ padding: ${theme.sizeUnit / 2}px;
+ border-radius: ${theme.borderRadius}px;
+ }
+ `}
+`;
+
+const HeaderContainer = styled.div`
+ ${({ theme }) => `
+ width: 100%;
+ display: flex;
+ align-items: center;
+ cursor: pointer;
+ padding: 0 ${theme.sizeUnit * 2}px;
+ overflow: hidden;
+ `}
+`;
+
+const HeaderLabel = styled.span`
+ ${({ theme }) => `
+ font-weight: ${theme.fontWeightStrong};
+ white-space: nowrap;
+ overflow: hidden;
+ text-overflow: ellipsis;
+ display: block;
+ max-width: 100%;
+ `}
+`;
+
+const SortIconWrapper = styled.div`
+ ${({ theme }) => `
+ display: flex;
+ align-items: center;
+ margin-left: ${theme.sizeUnit * 2}px;
+ `}
+`;
+
+const FilterIconWrapper = styled.div`
+ align-self: flex-end;
+ margin-left: auto;
+ cursor: pointer;
+`;
+
+const MenuContainer = styled.div`
+ ${({ theme }) => `
+ min-width: ${theme.sizeUnit * 45}px;
+ padding: ${theme.sizeUnit}px 0;
+
+ .menu-item {
+ padding: ${theme.sizeUnit * 2}px ${theme.sizeUnit * 4}px;
+ cursor: pointer;
+ display: flex;
+ align-items: center;
+ gap: ${theme.sizeUnit * 2}px;
+
+ &:hover {
+ background-color: ${theme.colors.primary.light4};
+ }
+ }
+
+ .menu-divider {
+ height: 1px;
+ background-color: ${theme.colors.grayscale.light2};
+ margin: ${theme.sizeUnit}px 0;
+ }
+ `}
+`;
+
+const getSortIcon = (
+ sortState: SortState[],
+ colId: string | null,
+): React.ReactNode => {
+ if (!sortState?.length || !colId) return null;
+ const currentSort = sortState[0];
+ if (currentSort.colId === colId) {
+ if (currentSort.sort === 'asc') return <ArrowUpOutlined />;
+ if (currentSort.sort === 'desc') return <ArrowDownOutlined />;
+ }
+ return null;
+};
+
+const CustomHeader: React.FC<CustomHeaderParams> = ({
+ displayName,
+ enableSorting,
+ setSort,
+ context,
+ column,
+ api,
+}) => {
+ const { initialSortState, onColumnHeaderClicked } = context;
+ const colId = column?.getColId();
+ const isPercentMetric = (column?.getColDef() as CustomColDef)?.context
+ ?.isPercentMetric;
+ const [isPopoverVisible, setPopoverVisible] = useState(false);
+ const filterContainerRef = useRef<HTMLDivElement>(null);
+ const [isMenuVisible, setIsMenuVisible] = useState(false);
+ const currentSort = initialSortState?.[0];
+
+ const userColumn = column.getUserProvidedColDef() as UserProvidedColDef;
+ const isMain = userColumn?.isMain;
+ const timeComparisonKey = userColumn?.timeComparisonKey || '';
+ const sortKey = isMain ? colId.replace('Main', '').trim() : colId;
+ const isTimeComparison = !isMain && timeComparisonKey;
+
+ const clearSort = () => {
+ onColumnHeaderClicked({ column: { colId: sortKey, sort: null } });
+ setSort(null, false);
+ };
+
+ const handleSortAsc = () => {
+ onColumnHeaderClicked({ column: { colId: sortKey, sort: 'asc' } });
+ setSort('asc', false);
+ };
+
+ const handleSortDesc = () => {
+ onColumnHeaderClicked({ column: { colId: sortKey, sort: 'desc' } });
+ setSort('desc', false);
+ };
Review Comment:
Can we dry this up a bit?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2891280183
Korbit doesn't automatically review large (3000+ lines changed) pull requests such as this one. If you want me to review anyway, use `/korbit-review`.
[](2c525575-0471-481e-88f8-94f371704703)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2096793773
##########
superset-frontend/packages/superset-ui-core/src/chart/types/VizType.ts:
##########
@@ -54,6 +54,7 @@ export enum VizType {
Step = 'echarts_timeseries_step',
Sunburst = 'sunburst_v2',
Table = 'table',
+ TableAgGrid = 'table_ag_grid',
Review Comment:
<img width="1358" alt="Screenshot 2025-05-20 at 8 45 57 AM" src="https://github.com/user-attachments/assets/912ce158-366d-46c0-98d1-3f3bb7cd154e" />
Another issue @justinpark @michael-s-molina , Any component that is used in viz plugin must be part of @superset-ui/core ... if we try to use a Component from src/components in viz plugin , we start getting errors , Should we move the Grid Table to superset-ui/core
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2979950769
@geido Ephemeral environment spinning up at http://34.219.160.161:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2166613721
##########
superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts:
##########
@@ -60,6 +60,7 @@ export enum FeatureFlag {
SlackEnableAvatars = 'SLACK_ENABLE_AVATARS',
EnableDashboardScreenshotEndpoints = 'ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS',
EnableDashboardDownloadWebDriverScreenshot = 'ENABLE_DASHBOARD_DOWNLOAD_WEBDRIVER_SCREENSHOT',
+ TableV2TimeComparisonEnabled = 'TABLE_V2_TIME_COMPARISON_ENABLED',
Review Comment:
I guess ,
There are 2 feature Flags for Ag Grid on Open source
TABLE_V2_TIME_COMPARISON_ENABLED <- This can be same across preset & Superset
CHART_PLUGINS_EXPERIMENTAL <- We shouldnt keep Ag Grid Under experimental as Interactive table has its own flags of ag Grid there
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-3035644253
@geido Ephemeral environment spinning up at http://35.166.80.66:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2966386594
@geido Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/15609751643). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2891269960
Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment `/korbit-review`.
> Your admin can change your review schedule in the [Korbit Console](https://app.korbit.ai/settings)
[](abea6735-abe3-4c7c-a85f-6eba04e694a5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2102974140
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx:
##########
@@ -0,0 +1,228 @@
+/* eslint-disable camelcase */
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ useCallback,
+ useMemo,
+ useRef,
+ memo,
+ useState,
+ ChangeEvent,
+} from 'react';
+
+import {
+ AllCommunityModule,
+ ClientSideRowModelModule,
+ type ColDef,
+ ModuleRegistry,
+ GridReadyEvent,
+ GridApi,
+} from 'ag-grid-community';
+import { AgGridReact } from 'ag-grid-react';
+import './styles/ag-grid.css';
+
+import { type FunctionComponent } from 'react';
+
+import { styled, css } from '@superset-ui/core';
+
+export interface Props {
+ gridTheme?: string;
+ isDarkMode?: boolean;
+ gridHeight?: number | null;
+ updateInterval?: number;
+ data?: any[];
+ onGridReady?: (params: GridReadyEvent) => void;
+ colDefsFromProps: any[];
+ includeSearch: boolean;
+ allowRearrangeColumns: boolean;
+ pagination: boolean;
+ pageSize: number;
+}
+
+ModuleRegistry.registerModules([AllCommunityModule, ClientSideRowModelModule]);
+
+const StyledContainer = styled.div`
+ ${({ theme }) => css`
+ .input-container {
+ margin-left: auto;
+ }
+ .input-wrapper {
+ position: relative;
+ display: flex;
+ align-items: center;
+ overflow: visible;
+ margin-bottom: 1rem;
+ }
+
+ .input-wrapper svg {
+ pointer-events: none;
+ transform: translate(28px, 2px);
+ color: ${theme.colors.grayscale.base};
+ }
+
+ .input-wrapper input {
+ font-size: ${theme.typography.sizes.s}px;
+ padding: 0.375em 0.75em 0.375em 2.4em;
+ line-height: 1.4;
+ border-radius: ${theme.gridUnit}px;
+ border: 1px solid ${theme.colors.grayscale.light2};
+ background-color: transparent;
+ outline: none;
+
+ &:focus {
+ border-color: ${theme.colors.primary.base};
+ }
+
+ &::placeholder {
+ color: ${theme.colors.grayscale.light1};
+ }
+ }
+ `}
+`;
+
+// Memoize the entire component
+const AgGridDataTable: FunctionComponent<Props> = memo(
+ ({
+ gridTheme = 'ag-theme-quartz',
+ isDarkMode = false,
+ gridHeight = null,
+ data = [],
+ onGridReady,
+ colDefsFromProps,
+ includeSearch,
+ allowRearrangeColumns,
+ pagination,
+ pageSize,
+ }) => {
+ const gridRef = useRef<AgGridReact>(null);
+ const gridApiRef = useRef<GridApi | null>(null);
+
+ const defaultColDef = useMemo<ColDef>(
+ () => ({
+ flex: 1,
+ filter: true,
+ enableRowGroup: true,
+ enableValue: true,
+ sortable: true,
+ resizable: true,
+ minWidth: 100,
+ }),
+ [],
+ );
+
+ // Handle grid ready event
+ const handleGridReady = useCallback(
+ (params: GridReadyEvent) => {
+ gridApiRef.current = params.api;
+ if (onGridReady) {
+ onGridReady(params);
+ }
+
+ // Optional: Auto-size columns on initial load
+ params.api.sizeColumnsToFit();
+ },
+ [onGridReady],
+ );
+
+ // Memoize container style
+ const containerStyle = useMemo(
+ () => ({
+ height: gridHeight
+ ? includeSearch
+ ? `${gridHeight - 16}px`
+ : `${gridHeight}px`
+ : '100%',
+ width: '100%',
+ }),
+ [gridHeight],
+ );
+
+ // Memoize grid class name
+ const gridClassName = useMemo(
+ () => `ag-theme-quartz${isDarkMode ? '-dark' : ''}`,
+ [isDarkMode],
+ );
+
+ const [quickFilterText, setQuickFilterText] = useState<string>();
+ const onFilterTextBoxChanged = useCallback(
+ ({ target: { value } }: ChangeEvent<HTMLInputElement>) =>
+ setQuickFilterText(value),
+ [],
+ );
+
+ console.log(pageSize, 'page size');
+
+ return (
+ <StyledContainer>
+ <div className={gridClassName} style={containerStyle}>
+ {includeSearch && (
+ <div className="input-wrapper">
+ <div className="input-container">
+ <svg
+ width="16"
+ height="16"
+ viewBox="0 0 16 16"
+ fill="none"
+ xmlns="http://www.w3.org/2000/svg"
+ >
+ <path
+ fillRule="evenodd"
+ clipRule="evenodd"
+ d="M11.5014 7.00039C11.5014 7.59133 11.385 8.1765 11.1588 8.72246C10.9327 9.26843 10.6012 9.7645 10.1833 10.1824C9.76548 10.6002 9.2694 10.9317 8.72344 11.1578C8.17747 11.384 7.59231 11.5004 7.00136 11.5004C6.41041 11.5004 5.82525 11.384 5.27929 11.1578C4.73332 10.9317 4.23725 10.6002 3.81938 10.1824C3.40152 9.7645 3.07005 9.26843 2.8439 8.72246C2.61776 8.1765 2.50136 7.59133 2.50136 7.00039C2.50136 5.80691 2.97547 4.66232 3.81938 3.81841C4.6633 2.97449 5.80789 2.50039 7.00136 2.50039C8.19484 2.50039 9.33943 2.97449 10.1833 3.81841C11.0273 4.66232 11.5014 5.80691 11.5014 7.00039ZM10.6814 11.7404C9.47574 12.6764 7.95873 13.1177 6.43916 12.9745C4.91959 12.8314 3.51171 12.1145 2.50211 10.9698C1.49252 9.8251 0.957113 8.33868 1.0049 6.81314C1.05268 5.28759 1.68006 3.83759 2.75932 2.75834C3.83857 1.67908 5.28856 1.0517 6.81411 1.00392C8.33966 0.956136 9.82608 1.49154 10.9708 2.50114C12.1154 3.51073 12.8323 4.91862 12.9755 6.43819C13.1187 7.95775 12.6773 9.47476 11.7414
10.6804L14.5314 13.4704C14.605 13.539 14.6642 13.6218 14.7051 13.7138C14.7461 13.8058 14.7682 13.9052 14.77 14.0059C14.7717 14.1066 14.7532 14.2066 14.7155 14.3C14.6778 14.3934 14.6216 14.4782 14.5504 14.5494C14.4792 14.6206 14.3943 14.6768 14.301 14.7145C14.2076 14.7522 14.1075 14.7708 14.0068 14.769C13.9061 14.7672 13.8068 14.7452 13.7148 14.7042C13.6228 14.6632 13.54 14.6041 13.4714 14.5304L10.6814 11.7404Z"
+ fill="currentColor"
+ />
+ </svg>
+
+ <input
+ type="text"
+ id="filter-text-box"
+ placeholder="Search"
+ onInput={onFilterTextBoxChanged}
+ />
+ </div>
+ </div>
+ )}
+
+ <AgGridReact
Review Comment:
> Should we move the Grid Table to superset-ui/core
Yes. If this component needs to be accessed by plugins it should be moved to `superset-ui/core`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2900937926
@geido Ephemeral environment spinning up at http://44.243.219.34:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2096201645
##########
superset-frontend/packages/superset-ui-core/src/chart/types/VizType.ts:
##########
@@ -54,6 +54,7 @@ export enum VizType {
Step = 'echarts_timeseries_step',
Sunburst = 'sunburst_v2',
Table = 'table',
+ TableAgGrid = 'table_ag_grid',
Review Comment:
Thanks for the PR @amaannawab923. Some questions from the PR review meeting
1 - Do we need separate visualizations or the intention should be to replace the current table with Ag Grid. The thinking here is that it would be nice to be able to support only one table in Superset. @michael-s-molina
2 - We should probably use `table_v2` instead of `table_ag_grid`. @sadpandajoe
3 - We should use the same component created for SQL Lab. @justinpark
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2096356005
##########
superset-frontend/packages/superset-ui-core/src/chart/types/VizType.ts:
##########
@@ -54,6 +54,7 @@ export enum VizType {
Step = 'echarts_timeseries_step',
Sunburst = 'sunburst_v2',
Table = 'table',
+ TableAgGrid = 'table_ag_grid',
Review Comment:
> We do need seperate visualisations since the original table has alot of time comparison features & already so much legacy code in place that rewriting the entire legacy table with ag grid can break the backward compatibility for existing clients , it might be possible in future to completely migrate to ag grid & discard the old table but as of now , i think this carries risk seeing the complexity of existing table chart , given that table is one of the most widely used charts
Thanks for the explanation. I agree that having separate visualizations with a potential future migration might diminish the risk here.
> 3.I thought the same thing in the first place , but the thing is, it will break the principle of loose coupling , sqllab & viz plugins have least common functionalities & to implement any functionality in viz plugin table v2 might cause implication in sqllab component where i have to keep on passing flags to check where isSqllab or isVizPlugin , so i implemented it from scratch so that its losely coupled & any changes in this component doesnt break in sqllab or sqlllab changes doesnt break viz plugin
Supporting different implementations will be problematic from a maintenance and user experience perspective. We should try to extract common functionality to a shareable component that can be extended to handle custom needs of SQL Lab and chart visualizations. Please collaborate with @justinpark on how we can reuse as much code as possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2166473244
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/package.json:
##########
@@ -27,6 +27,8 @@
"@react-icons/all-files": "^4.1.0",
"@types/d3-array": "^2.9.0",
"@types/react-table": "^7.7.20",
+ "ag-grid-community": "^33.1.1",
Review Comment:
Thats true , even i think so ... I tried to exclude it from peerDependencies but the ci cd would fail saying that this is an extraneous dependancy as per eslint rule & must be included in Peer Dependancy
That makes alot of sense of keeping the main Dep in @superset-ui/core and that would be a follow up pr where the core Component of Ag Grid will be moved to @superset-ui/core to be used by both Sqllab and Viz Plugins
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2167224275
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/styles/ag-grid.css:
##########
@@ -0,0 +1,6924 @@
+/* eslint-disable camelcase */
Review Comment:
I don't think this file is needed - we should use AG Grid's theming API instead
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2949520999
@geido Ephemeral environment spinning up at http://35.87.155.107:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2966680589
@geido Ephemeral environment spinning up at http://54.184.64.215:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-3031159374
@geido Ephemeral environment spinning up at http://52.38.186.187:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "rebecagbenevides (via GitHub)" <gi...@apache.org>.
rebecagbenevides commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-3036361635
Passed in QA for theming :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2096214540
##########
superset-frontend/packages/superset-ui-core/src/chart/types/VizType.ts:
##########
@@ -54,6 +54,7 @@ export enum VizType {
Step = 'echarts_timeseries_step',
Sunburst = 'sunburst_v2',
Table = 'table',
+ TableAgGrid = 'table_ag_grid',
Review Comment:
Thanks for the questions :
1. We do need seperate visualisations since the original table has alot of time comparison features & already so much legacy code in place that rewriting the entire legacy table with ag grid can break the backward compatibility for existing clients , it might be possible in future to completely migrate to ag grid & discard the old table but as of now , i think this carries risk seeing the complexity of existing table chart , given that table is one of the most widely used charts
2. Good point - even i think table_v2 would be better name , just needed a consensus from the pmcs
3.I thought the same thing in the first place , but the thing is, it will break the principle of loose coupling , sqllab & viz plugins have least common functionalities & to implement any functionality in viz plugin table v2 might cause implication in sqllab component where i have to keep on passing flags to check where isSqllab or isVizPlugin , so i implemented it from scratch so that its losely coupled & any changes in this component doesnt break in sqllab or sqlllab changes doesnt break viz plugin
@michael-s-molina @justinpark @sadpandajoe
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2096356005
##########
superset-frontend/packages/superset-ui-core/src/chart/types/VizType.ts:
##########
@@ -54,6 +54,7 @@ export enum VizType {
Step = 'echarts_timeseries_step',
Sunburst = 'sunburst_v2',
Table = 'table',
+ TableAgGrid = 'table_ag_grid',
Review Comment:
> We do need seperate visualisations since the original table has alot of time comparison features & already so much legacy code in place that rewriting the entire legacy table with ag grid can break the backward compatibility for existing clients , it might be possible in future to completely migrate to ag grid & discard the old table but as of now , i think this carries risk seeing the complexity of existing table chart , given that table is one of the most widely used charts
Thanks for the explanation. I agree that having separate visualizations with a potential future migration might diminish the risk here.
> 3.I thought the same thing in the first place , but the thing is, it will break the principle of loose coupling , sqllab & viz plugins have least common functionalities & to implement any functionality in viz plugin table v2 might cause implication in sqllab component where i have to keep on passing flags to check where isSqllab or isVizPlugin , so i implemented it from scratch so that its losely coupled & any changes in this component doesnt break in sqllab or sqlllab changes doesnt break viz plugin
Supporting different implementations will be problematic from a maintenance and user experience perspective. We should try to extract common functionality to a shareable component that can be extended to handle custom needs of SQL Lab and chart visualizations. Please collaborate with @justinpark on how we can reuse as much code as possible. He's the author of [[SIP-123] Proposal replacement of data table components with ag-grid](https://github.com/apache/superset/issues/27645) which is related with this work.
##########
superset-frontend/packages/superset-ui-core/src/chart/types/VizType.ts:
##########
@@ -54,6 +54,7 @@ export enum VizType {
Step = 'echarts_timeseries_step',
Sunburst = 'sunburst_v2',
Table = 'table',
+ TableAgGrid = 'table_ag_grid',
Review Comment:
> We do need seperate visualisations since the original table has alot of time comparison features & already so much legacy code in place that rewriting the entire legacy table with ag grid can break the backward compatibility for existing clients , it might be possible in future to completely migrate to ag grid & discard the old table but as of now , i think this carries risk seeing the complexity of existing table chart , given that table is one of the most widely used charts
Thanks for the explanation. I agree that having separate visualizations with a potential future migration might diminish the risk here.
> 3.I thought the same thing in the first place , but the thing is, it will break the principle of loose coupling , sqllab & viz plugins have least common functionalities & to implement any functionality in viz plugin table v2 might cause implication in sqllab component where i have to keep on passing flags to check where isSqllab or isVizPlugin , so i implemented it from scratch so that its losely coupled & any changes in this component doesnt break in sqllab or sqlllab changes doesnt break viz plugin
Supporting different implementations will be problematic from a maintenance and user experience perspective. We should try to extract common functionality to a shareable component that can be extended to handle custom needs of SQL Lab and chart visualizations. Please collaborate with @justinpark on how we can reuse as much code as possible. He's the author of [[SIP-123] Proposal replacement of data table components with ag-grid](https://github.com/apache/superset/issues/27645).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2096402075
##########
superset-frontend/packages/superset-ui-core/src/chart/types/VizType.ts:
##########
@@ -54,6 +54,7 @@ export enum VizType {
Step = 'echarts_timeseries_step',
Sunburst = 'sunburst_v2',
Table = 'table',
+ TableAgGrid = 'table_ag_grid',
Review Comment:
I happen to see this column level filters requirement as well from the original sip @justinpark , I will be happy to take this change & do it in Sqllab & in table viz plugin as well ... I guess that answers point number 2 from the above doubts
https://github.com/apache/superset/discussions/26364
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2096402075
##########
superset-frontend/packages/superset-ui-core/src/chart/types/VizType.ts:
##########
@@ -54,6 +54,7 @@ export enum VizType {
Step = 'echarts_timeseries_step',
Sunburst = 'sunburst_v2',
Table = 'table',
+ TableAgGrid = 'table_ag_grid',
Review Comment:
I happen to see this column level filters requirement as well from the original sip https://github.com/apache/superset/discussions/26364 @justinpark , I will be happy to take this change & do it in Sqllab & in table viz plugin as well ... I guess that answers point number 2 from the above doubts
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2966735833
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/33517?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 51.76%. Comparing base [(`76d897e`)](https://app.codecov.io/gh/apache/superset/commit/76d897eaa2f9e137102bc194c2e3109c29d0348f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`5ea2ea9`)](https://app.codecov.io/gh/apache/superset/commit/5ea2ea9f2b78a3ddf7d8cabda367388f0bb3b0dd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 1956 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #33517 +/- ##
==========================================
- Coverage 60.48% 51.76% -8.73%
==========================================
Files 1931 558 -1373
Lines 76236 40292 -35944
Branches 8568 4223 -4345
==========================================
- Hits 46114 20857 -25257
+ Misses 28017 18877 -9140
+ Partials 2105 558 -1547
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/33517/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/33517/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `47.22% <ø> (-1.94%)` | :arrow_down: |
| [javascript](https://app.codecov.io/gh/apache/superset/pull/33517/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/33517/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `50.97% <ø> (-2.83%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/33517/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `51.75% <ø> (-11.75%)` | :arrow_down: |
| [unit](https://app.codecov.io/gh/apache/superset/pull/33517/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (+42.36%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/33517?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "amaannawab923 (via GitHub)" <gi...@apache.org>.
amaannawab923 commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2102931451
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx:
##########
@@ -0,0 +1,228 @@
+/* eslint-disable camelcase */
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ useCallback,
+ useMemo,
+ useRef,
+ memo,
+ useState,
+ ChangeEvent,
+} from 'react';
+
+import {
+ AllCommunityModule,
+ ClientSideRowModelModule,
+ type ColDef,
+ ModuleRegistry,
+ GridReadyEvent,
+ GridApi,
+} from 'ag-grid-community';
+import { AgGridReact } from 'ag-grid-react';
+import './styles/ag-grid.css';
+
+import { type FunctionComponent } from 'react';
+
+import { styled, css } from '@superset-ui/core';
+
+export interface Props {
+ gridTheme?: string;
+ isDarkMode?: boolean;
+ gridHeight?: number | null;
+ updateInterval?: number;
+ data?: any[];
+ onGridReady?: (params: GridReadyEvent) => void;
+ colDefsFromProps: any[];
+ includeSearch: boolean;
+ allowRearrangeColumns: boolean;
+ pagination: boolean;
+ pageSize: number;
+}
+
+ModuleRegistry.registerModules([AllCommunityModule, ClientSideRowModelModule]);
+
+const StyledContainer = styled.div`
+ ${({ theme }) => css`
+ .input-container {
+ margin-left: auto;
+ }
+ .input-wrapper {
+ position: relative;
+ display: flex;
+ align-items: center;
+ overflow: visible;
+ margin-bottom: 1rem;
+ }
+
+ .input-wrapper svg {
+ pointer-events: none;
+ transform: translate(28px, 2px);
+ color: ${theme.colors.grayscale.base};
+ }
+
+ .input-wrapper input {
+ font-size: ${theme.typography.sizes.s}px;
+ padding: 0.375em 0.75em 0.375em 2.4em;
+ line-height: 1.4;
+ border-radius: ${theme.gridUnit}px;
+ border: 1px solid ${theme.colors.grayscale.light2};
+ background-color: transparent;
+ outline: none;
+
+ &:focus {
+ border-color: ${theme.colors.primary.base};
+ }
+
+ &::placeholder {
+ color: ${theme.colors.grayscale.light1};
+ }
+ }
+ `}
+`;
+
+// Memoize the entire component
+const AgGridDataTable: FunctionComponent<Props> = memo(
+ ({
+ gridTheme = 'ag-theme-quartz',
+ isDarkMode = false,
+ gridHeight = null,
+ data = [],
+ onGridReady,
+ colDefsFromProps,
+ includeSearch,
+ allowRearrangeColumns,
+ pagination,
+ pageSize,
+ }) => {
+ const gridRef = useRef<AgGridReact>(null);
+ const gridApiRef = useRef<GridApi | null>(null);
+
+ const defaultColDef = useMemo<ColDef>(
+ () => ({
+ flex: 1,
+ filter: true,
+ enableRowGroup: true,
+ enableValue: true,
+ sortable: true,
+ resizable: true,
+ minWidth: 100,
+ }),
+ [],
+ );
+
+ // Handle grid ready event
+ const handleGridReady = useCallback(
+ (params: GridReadyEvent) => {
+ gridApiRef.current = params.api;
+ if (onGridReady) {
+ onGridReady(params);
+ }
+
+ // Optional: Auto-size columns on initial load
+ params.api.sizeColumnsToFit();
+ },
+ [onGridReady],
+ );
+
+ // Memoize container style
+ const containerStyle = useMemo(
+ () => ({
+ height: gridHeight
+ ? includeSearch
+ ? `${gridHeight - 16}px`
+ : `${gridHeight}px`
+ : '100%',
+ width: '100%',
+ }),
+ [gridHeight],
+ );
+
+ // Memoize grid class name
+ const gridClassName = useMemo(
+ () => `ag-theme-quartz${isDarkMode ? '-dark' : ''}`,
+ [isDarkMode],
+ );
+
+ const [quickFilterText, setQuickFilterText] = useState<string>();
+ const onFilterTextBoxChanged = useCallback(
+ ({ target: { value } }: ChangeEvent<HTMLInputElement>) =>
+ setQuickFilterText(value),
+ [],
+ );
+
+ console.log(pageSize, 'page size');
+
+ return (
+ <StyledContainer>
+ <div className={gridClassName} style={containerStyle}>
+ {includeSearch && (
+ <div className="input-wrapper">
+ <div className="input-container">
+ <svg
+ width="16"
+ height="16"
+ viewBox="0 0 16 16"
+ fill="none"
+ xmlns="http://www.w3.org/2000/svg"
+ >
+ <path
+ fillRule="evenodd"
+ clipRule="evenodd"
+ d="M11.5014 7.00039C11.5014 7.59133 11.385 8.1765 11.1588 8.72246C10.9327 9.26843 10.6012 9.7645 10.1833 10.1824C9.76548 10.6002 9.2694 10.9317 8.72344 11.1578C8.17747 11.384 7.59231 11.5004 7.00136 11.5004C6.41041 11.5004 5.82525 11.384 5.27929 11.1578C4.73332 10.9317 4.23725 10.6002 3.81938 10.1824C3.40152 9.7645 3.07005 9.26843 2.8439 8.72246C2.61776 8.1765 2.50136 7.59133 2.50136 7.00039C2.50136 5.80691 2.97547 4.66232 3.81938 3.81841C4.6633 2.97449 5.80789 2.50039 7.00136 2.50039C8.19484 2.50039 9.33943 2.97449 10.1833 3.81841C11.0273 4.66232 11.5014 5.80691 11.5014 7.00039ZM10.6814 11.7404C9.47574 12.6764 7.95873 13.1177 6.43916 12.9745C4.91959 12.8314 3.51171 12.1145 2.50211 10.9698C1.49252 9.8251 0.957113 8.33868 1.0049 6.81314C1.05268 5.28759 1.68006 3.83759 2.75932 2.75834C3.83857 1.67908 5.28856 1.0517 6.81411 1.00392C8.33966 0.956136 9.82608 1.49154 10.9708 2.50114C12.1154 3.51073 12.8323 4.91862 12.9755 6.43819C13.1187 7.95775 12.6773 9.47476 11.7414
10.6804L14.5314 13.4704C14.605 13.539 14.6642 13.6218 14.7051 13.7138C14.7461 13.8058 14.7682 13.9052 14.77 14.0059C14.7717 14.1066 14.7532 14.2066 14.7155 14.3C14.6778 14.3934 14.6216 14.4782 14.5504 14.5494C14.4792 14.6206 14.3943 14.6768 14.301 14.7145C14.2076 14.7522 14.1075 14.7708 14.0068 14.769C13.9061 14.7672 13.8068 14.7452 13.7148 14.7042C13.6228 14.6632 13.54 14.6041 13.4714 14.5304L10.6814 11.7404Z"
+ fill="currentColor"
+ />
+ </svg>
+
+ <input
+ type="text"
+ id="filter-text-box"
+ placeholder="Search"
+ onInput={onFilterTextBoxChanged}
+ />
+ </div>
+ </div>
+ )}
+
+ <AgGridReact
Review Comment:
Any component that is used in viz plugin must be part of @superset-ui/core ... if we try to use a Component from src/components in viz plugin , we start getting errors , Should we move the Grid Table to superset-ui/core
I am unable to use src/components/GridTable in plugins directory ,
Also none of the viz plugins use in Plugins directory ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2103032376
##########
superset-frontend/packages/superset-ui-core/src/chart/types/VizType.ts:
##########
@@ -54,6 +54,7 @@ export enum VizType {
Step = 'echarts_timeseries_step',
Sunburst = 'sunburst_v2',
Table = 'table',
+ TableAgGrid = 'table_ag_grid',
Review Comment:
It seems this component is well-written and doesn't have access to Superset's internal state directly like Redux calls. There's currently an effort from Preset to move many components from `src/components` to `@superset-ui/core` to be reusable by plugins and later by extensions. This seems like a perfect example of a valuable component that needs to be in `@superset-ui/core`. In case, there are any references to Redux or Superset's specific things, we just need to extract them from the component to a HOC and keep the component as a pure UI element before moving to `@superset-ui/core`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "justinpark (via GitHub)" <gi...@apache.org>.
justinpark commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2102980064
##########
superset-frontend/packages/superset-ui-core/src/chart/types/VizType.ts:
##########
@@ -54,6 +54,7 @@ export enum VizType {
Step = 'echarts_timeseries_step',
Sunburst = 'sunburst_v2',
Table = 'table',
+ TableAgGrid = 'table_ag_grid',
Review Comment:
@amaannawab923 The purpose of creating GridTable is to expand additional features such as those required in the current situation, along with version control, in order to reuse the added functionalities in other components that are in progress, as suggested in the [SIP](https://github.com/apache/superset/issues/27645). The parts of src/component/GridTable that cannot be imported in the superset viz plugin can be migrated from the existing grid-table to superset-core-ui, making it possible. If you need assistance with this, please let me know.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33517:
URL: https://github.com/apache/superset/pull/33517#issuecomment-2970311672
@geido Ephemeral environment spinning up at http://54.184.36.54:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(viz-type): Ag grid table plugin Integration [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on code in PR #33517:
URL: https://github.com/apache/superset/pull/33517#discussion_r2144925493
##########
superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts:
##########
@@ -58,6 +58,7 @@ export enum FeatureFlag {
SlackEnableAvatars = 'SLACK_ENABLE_AVATARS',
EnableDashboardScreenshotEndpoints = 'ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS',
EnableDashboardDownloadWebDriverScreenshot = 'ENABLE_DASHBOARD_DOWNLOAD_WEBDRIVER_SCREENSHOT',
+ TableV2TimeComparisonEnabled = 'TABLE_V2_TIME_COMPARISON_ENABLED',
Review Comment:
Do we have one for the standard one too? I think we might want to hide the Ag Grid implementation behind feature flag while it is being adopted by the community
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org