kokhlo commented on code in PR #44982:
URL: https://github.com/apache/superset/pull/44982#discussion_r4241122890
##########
superset-frontend/src/dashboard/components/RefreshFrequency/RefreshFrequencySelect.tsx:
##########
@@ -100,24 +166,48 @@ export const RefreshFrequencySelect = ({
value,
onChange,
}: RefreshFrequencySelectProps) => {
+ const configuredIntervals = useSelector(
+ (state: RootState) =>
+ // The select also mounts from the Dashboard List / Home pages, where no
+ // dashboard has been hydrated yet; the bootstrapped store carries the
+ // same config there.
+ state.dashboardInfo?.common?.conf?.DASHBOARD_AUTO_REFRESH_INTERVALS ??
+ state.common?.conf?.DASHBOARD_AUTO_REFRESH_INTERVALS,
+ );
+ const options = useMemo(
+ () => getRefreshFrequencyOptions(configuredIntervals),
+ [configuredIntervals],
+ );
+ const presets = options.filter(
+ option => option.value !== CUSTOM_REFRESH_FREQUENCY,
+ );
+
// Separate radio selection state from value state
const [radioSelection, setRadioSelection] = useState(() =>
- isPresetValue(value) ? value : -1,
+ isPresetValue(value, options) ? value : CUSTOM_REFRESH_FREQUENCY,
);
- const [customValue, setCustomValue] = useState(() => getCustomValue(value));
+ const [customValue, setCustomValue] = useState(() =>
+ getCustomValue(value, options),
+ );
useEffect(() => {
- const selection = isPresetValue(value) ? value : -1;
+ const selection = isPresetValue(value, options)
+ ? value
+ : CUSTOM_REFRESH_FREQUENCY;
setRadioSelection(selection);
Review Comment:
Addressed in 7090b0ebff. The sync effect no longer infers the selection from
preset membership alone: `handleRadioChange` (Custom branch) and
`handleCustomInputChange` record the last value they emitted in a ref, and when
the parent echoes that value back, the effect keeps Custom selected and shows
the value itself in the input instead of letting `getCustomValue` blank a
preset-colliding number. Selecting a preset still clears the ref, so preset
clicks snap to the preset as before.
Pinned by `Custom stays selected when the emitted value collides with a
preset` (exact repro: intervals `[[0, ...], [1, '1 second'], [600, ...]]`,
initial value 0, click Custom — asserts the `1 second` radio stays unchecked,
Custom stays checked, the input stays enabled at 1), plus the typed-variant
(typing 600 into Custom while a `10 minutes` preset exists) and a guard that
preset clicks still win.
--
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]