pierrejeambrun commented on code in PR #53151:
URL: https://github.com/apache/airflow/pull/53151#discussion_r2207761885
##########
airflow-core/src/airflow/ui/src/layouts/Details/DetailsLayout.tsx:
##########
@@ -68,6 +76,61 @@ export const DetailsLayout = ({ children, error, isLoading,
tabs }: Props) => {
const { onClose, onOpen, open } = useDisclosure();
const [isRightPanelCollapsed, setIsRightPanelCollapsed] = useState(false);
+ const getDefaultSizes = useCallback(
+ (): PanelSizes => (dagView === "graph" ? DEFAULT_GRAPH_SIZES :
DEFAULT_GRID_SIZES),
+ [dagView],
+ );
+
+ const getSavedSizes = useCallback((): PanelSizes => {
+ const storageKey = `panel-${dagId}-${dagView}`;
+
+ try {
+ const cached = sessionStorage.getItem(storageKey);
+
+ if (cached !== null && cached !== "") {
+ const parsed: unknown = JSON.parse(cached);
+
+ if (isPanelSizes(parsed)) {
+ return parsed;
+ }
+ }
+ } catch {
+ // Failed to parse saved panel sizes - fallback to default
+ }
+
+ return getDefaultSizes();
+ }, [dagId, dagView, getDefaultSizes]);
+
+ const savePanelSizes = useCallback(
+ (sizes: Array<number>) => {
+ try {
+ const storageKey = `panel-${dagId}-${dagView}`;
+
+ sessionStorage.setItem(storageKey, JSON.stringify(sizes));
+ } catch {
+ // Failed to save panel sizes - continue silently
+ }
+ },
+ [dagId, dagView],
+ );
+
+ const [currentSizes, setCurrentSizes] = useState<PanelSizes>(getSavedSizes);
+
+ const [showPanel, setShowPanel] = useState(false);
+
+ useEffect(() => {
+ const newSizes = getSavedSizes();
+
+ setCurrentSizes(newSizes);
+
+ setShowPanel(false);
+ const timer = setTimeout(() => {
+ setShowPanel(true);
+ }, PANEL_RESET_DELAY);
+
+ return () => clearTimeout(timer);
+ }, [dagView, dagId, getSavedSizes]);
Review Comment:
Ideally we would like a solution without useEffect. Is there a way to do it
differently ?
##########
airflow-core/src/airflow/ui/src/layouts/Details/DetailsLayout.tsx:
##########
@@ -68,6 +76,61 @@ export const DetailsLayout = ({ children, error, isLoading,
tabs }: Props) => {
const { onClose, onOpen, open } = useDisclosure();
const [isRightPanelCollapsed, setIsRightPanelCollapsed] = useState(false);
+ const getDefaultSizes = useCallback(
+ (): PanelSizes => (dagView === "graph" ? DEFAULT_GRAPH_SIZES :
DEFAULT_GRID_SIZES),
+ [dagView],
+ );
+
+ const getSavedSizes = useCallback((): PanelSizes => {
+ const storageKey = `panel-${dagId}-${dagView}`;
+
+ try {
+ const cached = sessionStorage.getItem(storageKey);
+
+ if (cached !== null && cached !== "") {
+ const parsed: unknown = JSON.parse(cached);
+
+ if (isPanelSizes(parsed)) {
+ return parsed;
+ }
+ }
+ } catch {
+ // Failed to parse saved panel sizes - fallback to default
+ }
Review Comment:
I don't think we can have a failure reeading that. And if we do, that's not
normal and we might want to break there instead of silently returning anything.
##########
airflow-core/src/airflow/ui/src/layouts/Details/DetailsLayout.tsx:
##########
@@ -68,6 +76,61 @@ export const DetailsLayout = ({ children, error, isLoading,
tabs }: Props) => {
const { onClose, onOpen, open } = useDisclosure();
const [isRightPanelCollapsed, setIsRightPanelCollapsed] = useState(false);
+ const getDefaultSizes = useCallback(
+ (): PanelSizes => (dagView === "graph" ? DEFAULT_GRAPH_SIZES :
DEFAULT_GRID_SIZES),
+ [dagView],
+ );
+
+ const getSavedSizes = useCallback((): PanelSizes => {
+ const storageKey = `panel-${dagId}-${dagView}`;
+
+ try {
+ const cached = sessionStorage.getItem(storageKey);
+
+ if (cached !== null && cached !== "") {
+ const parsed: unknown = JSON.parse(cached);
+
+ if (isPanelSizes(parsed)) {
+ return parsed;
+ }
+ }
+ } catch {
+ // Failed to parse saved panel sizes - fallback to default
+ }
+
+ return getDefaultSizes();
+ }, [dagId, dagView, getDefaultSizes]);
+
+ const savePanelSizes = useCallback(
+ (sizes: Array<number>) => {
+ try {
+ const storageKey = `panel-${dagId}-${dagView}`;
+
+ sessionStorage.setItem(storageKey, JSON.stringify(sizes));
+ } catch {
+ // Failed to save panel sizes - continue silently
+ }
Review Comment:
What are we trying to catch there? This shouldn't fail.
##########
airflow-core/src/airflow/ui/src/layouts/Details/DetailsLayout.tsx:
##########
@@ -44,6 +44,15 @@ import { Grid } from "./Grid";
import { NavTabs } from "./NavTabs";
import { PanelButtons } from "./PanelButtons";
+const PANEL_RESET_DELAY = 10;
+const DEFAULT_GRAPH_SIZES = [70, 30] as const;
+const DEFAULT_GRID_SIZES = [20, 80] as const;
+
+type PanelSizes = readonly [number, number];
+
+const isPanelSizes = (value: unknown): value is PanelSizes =>
+ Array.isArray(value) && value.length === 2 && value.every((size) => typeof
size === "number" && size > 0);
Review Comment:
Why do we need 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
