rusackas commented on code in PR #37902:
URL: https://github.com/apache/superset/pull/37902#discussion_r2796447937
##########
superset-frontend/src/components/Chart/Chart.tsx:
##########
@@ -176,243 +163,310 @@ const MessageSpan = styled.span`
color: ${({ theme }) => theme.colorText};
`;
-class Chart extends PureComponent<ChartProps, {}> {
- static defaultProps = defaultProps;
-
- renderStartTime: any;
-
- constructor(props: ChartProps) {
- super(props);
- this.handleRenderContainerFailure =
- this.handleRenderContainerFailure.bind(this);
- }
-
- componentDidMount() {
- if (this.props.triggerQuery) {
- this.runQuery();
- }
- }
-
- componentDidUpdate() {
- if (this.props.triggerQuery) {
- this.runQuery();
- }
- }
-
- shouldRenderChart() {
- return (
- this.props.isInView ||
+function Chart({
+ addFilter = () => BLANK,
+ onFilterMenuOpen = () => BLANK,
+ onFilterMenuClose = () => BLANK,
+ initialValues = BLANK,
+ setControlValue = () => BLANK,
+ triggerRender = false,
+ dashboardId,
+ chartStackTrace,
+ force = false,
+ isInView = true,
+ ...restProps
+}: ChartProps): JSX.Element {
+ const {
+ actions,
+ chartId,
+ datasource,
+ formData,
+ timeout,
+ ownState,
+ chartAlert,
+ chartStatus,
+ queriesResponse = [],
+ errorMessage,
+ chartIsStale,
+ width,
+ height,
+ datasetsStatus,
+ onQuery,
+ annotationData,
+ labelColors: _labelColors,
+ sharedLabelColors: _sharedLabelColors,
+ vizType,
+ isFiltersInitialized: _isFiltersInitialized,
+ latestQueryFormData,
+ triggerQuery,
+ postTransformProps,
+ emitCrossFilters,
+ onChartStateChange,
+ } = restProps;
+
+ const renderStartTimeRef = useRef<number>(Logger.getTimestamp());
+
Review Comment:
Fixed in 72ed19d325. Added `renderStartTimeRef.current =
Logger.getTimestamp();` at the top of the render to update the timestamp on
each render, enabling accurate duration tracking when errors occur.
##########
superset-frontend/src/dashboard/components/Dashboard.tsx:
##########
@@ -321,24 +217,144 @@ class Dashboard extends PureComponent<DashboardProps> {
});
// remove dup in affectedChartIds
- this.refreshCharts([...new Set(affectedChartIds)]);
- this.appliedFilters = activeFilters;
- this.appliedOwnDataCharts = ownDataCharts;
- }
+ refreshCharts([...new Set(affectedChartIds)]);
+ appliedFiltersRef.current = activeFilters;
+ appliedOwnDataChartsRef.current = ownDataCharts;
+ }, [activeFilters, ownDataCharts, slices, refreshCharts]);
- refreshCharts(ids: (string | number)[]): void {
- ids.forEach(id => {
- this.props.actions.triggerQuery(true, id);
- });
- }
+ const applyCharts = useCallback((): void => {
+ if (!chartConfiguration) {
+ // For a first loading we need to wait for cross filters charts data
loaded to get all active filters
+ // for correct comparing of filters to avoid unnecessary requests
+ return;
+ }
+
+ if (
+ !editMode &&
+ (!areObjectsEqual(appliedOwnDataChartsRef.current, ownDataCharts, {
+ ignoreUndefined: true,
+ }) ||
+ !areObjectsEqual(appliedFiltersRef.current, activeFilters, {
+ ignoreUndefined: true,
+ }))
+ ) {
+ applyFilters();
+ }
- render(): ReactNode {
- const context = this.context as PluginContextType;
- if (context.loading) {
- return <Loading />;
+ if (hasUnsavedChanges) {
+ onBeforeUnload(true);
+ } else {
+ onBeforeUnload(false);
}
- return this.props.children;
+ }, [
+ chartConfiguration,
+ editMode,
+ ownDataCharts,
+ activeFilters,
+ hasUnsavedChanges,
+ applyFilters,
+ ]);
+
+ const onVisibilityChange = useCallback((): void => {
+ if (document.visibilityState === 'hidden') {
+ // from visible to hidden
+ visibilityEventDataRef.current = {
+ start_offset: Logger.getTimestamp(),
+ ts: new Date().getTime(),
+ };
+ } else if (document.visibilityState === 'visible') {
+ // from hidden to visible
+ const logStart = visibilityEventDataRef.current.start_offset;
+ actions.logEvent(LOG_ACTIONS_HIDE_BROWSER_TAB, {
+ ...visibilityEventDataRef.current,
+ duration: Logger.getTimestamp() - logStart,
+ });
+ }
+ }, [actions]);
+
+ // componentDidMount equivalent
+ useEffect(() => {
+ const bootstrapData = getBootstrapData();
+ const eventData: Record<string, unknown> = {
+ is_soft_navigation: Logger.timeOriginOffset > 0,
+ is_edit_mode: editMode,
+ mount_duration: Logger.getTimestamp(),
+ is_empty: isDashboardEmpty(layout),
+ is_published: isPublished,
+ bootstrap_data_length: JSON.stringify(bootstrapData).length,
+ };
+ const directLinkComponentId = getLocationHash();
+ if (directLinkComponentId) {
+ eventData.target_id = directLinkComponentId;
+ }
+ actions.logEvent(LOG_ACTIONS_MOUNT_DASHBOARD, eventData);
+
+ // Handle browser tab visibility change
+ if (document.visibilityState === 'hidden') {
+ visibilityEventDataRef.current = {
+ start_offset: Logger.getTimestamp(),
+ ts: new Date().getTime(),
+ };
+ }
+ window.addEventListener('visibilitychange', onVisibilityChange);
+
+ // componentWillUnmount equivalent
+ return () => {
+ window.removeEventListener('visibilitychange', onVisibilityChange);
Review Comment:
Fixed in 72ed19d325. Added `onBeforeUnload(false)` in the cleanup function
to remove the beforeunload listener when the component unmounts:
```typescript
return () => {
window.removeEventListener('visibilitychange', onVisibilityChange);
onBeforeUnload(false); // Remove beforeunload listener on unmount
actions.clearDataMaskState();
actions.clearAllChartStates();
};
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
