aminghadersohi commented on code in PR #44878:
URL: https://github.com/apache/superset/pull/44878#discussion_r4423324012
##########
superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx:
##########
@@ -1733,6 +1725,286 @@ test('does not request datasource metadata when an
unrelated control changes', (
}
});
+type Axis = 'x' | 'y';
+
+// Explore state for a Table chart whose `<axis>_axis_title` and
+// `<axis>_axis_title_margin` controls start at the given values.
+const axisTitleState = (axis: Axis, title: string, margin: number) => ({
+ ...reduxState,
+ explore: {
+ ...reduxState.explore,
+ form_data: {
+ datasource: '1__table',
+ viz_type: VizType.Table,
+ metrics: [],
+ },
+ controls: {
+ ...reduxState.explore.controls,
+ [`${axis}_axis_title`]: { value: title },
+ [`${axis}_axis_title_margin`]: { value: margin },
+ },
+ },
+});
+
+// The arguments of a programmatic margin write made by the container itself,
+// as opposed to the dispatches the tests make.
+const programmaticMarginWrite = (axis: Axis) => [
+ `${axis}_axis_title_margin`,
+ expect.anything(),
+ undefined,
+ { programmatic: true },
+];
+
+const controlValue = (store: Store, name: string) =>
+ (
+ store.getState() as unknown as {
+ explore: { controls: Record<string, { value: unknown }> };
+ }
+ ).explore.controls[name].value;
+
+const marginIn = (store: Store, axis: Axis) =>
+ controlValue(store, `${axis}_axis_title_margin`);
+
+test.each<Axis>(['x', 'y'])(
+ 'automatic %s axis title margin adjustment ignores a transient empty value
from a quick clear-and-retype',
+ async axis => {
+ setupTableChartControlPanel();
+ try {
+ const setControlValueSpy = jest.spyOn(exploreActions, 'setControlValue');
+
+ // a custom margin above the default 30
+ const initialState = axisTitleState(axis, 'Axis Label', 75);
+ const store = createStore(initialState, reducerIndex);
+ renderWithRouter({ initialState, store: store as Store });
+
+ // Clear any calls from initial render
+ setControlValueSpy.mockClear();
+
+ // TextControl now reports edits via useDeferredValue instead of a fixed
+ // debounce, so a quick clear-and-retype (deleting a title and
+ // immediately typing a replacement) can briefly pass an empty value
+ // through here before the retyped value settles. The margin effect
+ // must react to the settled value only, not this transient one, or it
+ // would zero out a custom margin and then bump it back to 30, losing
+ // the original 75.
Review Comment:
Nit: Same stale rationale as the comment fixed in `index.tsx`: TextControl
still settles on a trailing `FAST_DEBOUNCE`, so the transient empty title comes
from a paused clear or a blur flush.
```suggestion
// A transient axis title -- e.g. one cleared and then replaced, or one
// flushed on blur mid-edit -- can briefly pass an empty value through
// here before the retyped value settles. The margin effect must react
// to the settled value only, not this transient one, or it would zero
// out a custom margin and then bump it back to 30, losing the original
// 75.
```
--
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]
