-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: persist window frames and widths #7409
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
c87c551
55b35ae
018ecdd
e4d6866
7a2afcb
ba72632
0d87a41
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,6 +7,7 @@ import StyledWrapper from './StyledWrapper'; | |||||||||
| const MIN_COLUMN_WIDTH = 80; | ||||||||||
|
|
||||||||||
| const EditableTable = ({ | ||||||||||
| tableId, // Not being used kept to maintain uniqueness & pass similar in onColumnWidthsChange | ||||||||||
| columns, | ||||||||||
| rows, | ||||||||||
| onChange, | ||||||||||
|
|
@@ -20,21 +21,35 @@ const EditableTable = ({ | |||||||||
| reorderable = false, | ||||||||||
| onReorder, | ||||||||||
| showAddRow = true, | ||||||||||
| testId = 'editable-table' | ||||||||||
| testId = 'editable-table', | ||||||||||
| columnWidths, | ||||||||||
| onColumnWidthsChange | ||||||||||
| }) => { | ||||||||||
| const tableRef = useRef(null); | ||||||||||
| const emptyRowUidRef = useRef(null); | ||||||||||
| const [hoveredRow, setHoveredRow] = useState(null); | ||||||||||
| const [resizing, setResizing] = useState(null); | ||||||||||
| const [tableHeight, setTableHeight] = useState(0); | ||||||||||
| const [columnWidths, setColumnWidths] = useState(() => { | ||||||||||
| const [localColumnWidths, setLocalColumnWidths] = useState(() => { | ||||||||||
| const initialWidths = {}; | ||||||||||
| columns.forEach((col) => { | ||||||||||
| initialWidths[col.key] = col.width || 'auto'; | ||||||||||
| }); | ||||||||||
| return initialWidths; | ||||||||||
| }); | ||||||||||
|
|
||||||||||
| // Use controlled props if provided, otherwise use local state | ||||||||||
| const isControlled = columnWidths !== undefined; | ||||||||||
| const widths = isControlled ? columnWidths : localColumnWidths; | ||||||||||
|
|
||||||||||
| const handleColumnWidthsChange = useCallback((newWidths) => { | ||||||||||
| if (isControlled && onColumnWidthsChange) { | ||||||||||
| onColumnWidthsChange(newWidths); | ||||||||||
| } else { | ||||||||||
| setLocalColumnWidths(newWidths); | ||||||||||
| } | ||||||||||
| }, [isControlled, onColumnWidthsChange]); | ||||||||||
|
|
||||||||||
| const handleResizeStart = useCallback((e, columnKey) => { | ||||||||||
| e.preventDefault(); | ||||||||||
| e.stopPropagation(); | ||||||||||
|
|
@@ -59,11 +74,15 @@ const EditableTable = ({ | |||||||||
| const maxShrink = startWidth - MIN_COLUMN_WIDTH; | ||||||||||
| const clampedDiff = Math.max(-maxShrink, Math.min(maxGrow, diff)); | ||||||||||
|
|
||||||||||
| setColumnWidths((prev) => ({ | ||||||||||
| ...prev, | ||||||||||
| const newWidths = { | ||||||||||
| ...widths, | ||||||||||
| [columnKey]: `${startWidth + clampedDiff}px`, | ||||||||||
| [nextColumnKey]: `${nextColumnStartWidth - clampedDiff}px` | ||||||||||
| })); | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| if (isControlled) { | ||||||||||
| handleColumnWidthsChange(newWidths); | ||||||||||
| } | ||||||||||
|
Comment on lines
+83
to
+85
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Uncontrolled mode resize is broken.
🐛 Proposed fix- if (isControlled) {
- handleColumnWidthsChange(newWidths);
- }
+ handleColumnWidthsChange(newWidths);Apply the same fix at line 111: - if (isControlled) {
- handleColumnWidthsChange({ ...widths, ...newWidths });
- }
+ handleColumnWidthsChange({ ...widths, ...newWidths });📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
| }; | ||||||||||
|
|
||||||||||
| const handleMouseUp = () => { | ||||||||||
|
|
@@ -88,7 +107,9 @@ const EditableTable = ({ | |||||||||
| }); | ||||||||||
|
|
||||||||||
| if (Object.keys(newWidths).length > 0) { | ||||||||||
| setColumnWidths((prev) => ({ ...prev, ...newWidths })); | ||||||||||
| if (isControlled) { | ||||||||||
| handleColumnWidthsChange({ ...widths, ...newWidths }); | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| setResizing(null); | ||||||||||
|
|
@@ -98,7 +119,7 @@ const EditableTable = ({ | |||||||||
|
|
||||||||||
| document.addEventListener('mousemove', handleMouseMove); | ||||||||||
| document.addEventListener('mouseup', handleMouseUp); | ||||||||||
| }, [columns, showCheckbox]); | ||||||||||
| }, [columns, showCheckbox, widths, isControlled, handleColumnWidthsChange]); | ||||||||||
|
|
||||||||||
| // Track table height for resize handles | ||||||||||
| useEffect(() => { | ||||||||||
|
|
@@ -118,8 +139,8 @@ const EditableTable = ({ | |||||||||
| }, [rows.length]); | ||||||||||
|
|
||||||||||
| const getColumnWidth = useCallback((column) => { | ||||||||||
| return columnWidths[column.key] || column.width || 'auto'; | ||||||||||
| }, [columnWidths]); | ||||||||||
| return widths[column.key] || column.width || 'auto'; | ||||||||||
| }, [widths]); | ||||||||||
|
|
||||||||||
| const createEmptyRow = useCallback(() => { | ||||||||||
| const newUid = uuid(); | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,7 +3,8 @@ import { TableVirtuoso } from 'react-virtuoso'; | |||||
| import cloneDeep from 'lodash/cloneDeep'; | ||||||
| import { IconTrash, IconAlertCircle, IconInfoCircle } from '@tabler/icons'; | ||||||
| import { useTheme } from 'providers/Theme'; | ||||||
| import { useSelector } from 'react-redux'; | ||||||
| import { useSelector, useDispatch } from 'react-redux'; | ||||||
| import { updateTableColumnWidths } from 'providers/ReduxStore/slices/tabs'; | ||||||
| import MultiLineEditor from 'components/MultiLineEditor/index'; | ||||||
| import StyledWrapper from './StyledWrapper'; | ||||||
| import { uuid } from 'utils/common'; | ||||||
|
|
@@ -55,13 +56,37 @@ const EnvironmentVariablesTable = ({ | |||||
| const valueMatchBg = theme?.colors?.accent ? `${theme.colors.accent}1a` : undefined; | ||||||
| const { globalEnvironments, activeGlobalEnvironmentUid } = useSelector((state) => state.globalEnvironments); | ||||||
|
|
||||||
| const dispatch = useDispatch(); | ||||||
| const tabs = useSelector((state) => state.tabs.tabs); | ||||||
| const activeTabUid = useSelector((state) => state.tabs.activeTabUid); | ||||||
|
|
||||||
| const hasDraftForThisEnv = draft?.environmentUid === environment.uid; | ||||||
|
|
||||||
| const [tableHeight, setTableHeight] = useState(MIN_H); | ||||||
| const [columnWidths, setColumnWidths] = useState({ name: '30%', value: 'auto' }); | ||||||
|
|
||||||
| // Use environment UID as part of tableId so each environment has its own column widths | ||||||
| const tableId = `env-vars-table-${environment.uid}`; | ||||||
|
|
||||||
| // Get column widths from Redux - derived value (not state) | ||||||
| const focusedTab = tabs?.find((t) => t.uid === activeTabUid); | ||||||
| const storedColumnWidths = focusedTab?.tableColumnWidths?.[tableId]; | ||||||
|
|
||||||
| // Local state initialized from Redux (computed once on mount/environment change via key) | ||||||
| const [columnWidths, setColumnWidths] = useState(() => { | ||||||
| return storedColumnWidths || { name: '30%', value: 'auto' }; | ||||||
| }); | ||||||
|
Comment on lines
+70
to
+77
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sync This state only reads Suggested fix const [columnWidths, setColumnWidths] = useState(() => {
return storedColumnWidths || { name: '30%', value: 'auto' };
});
+
+ useEffect(() => {
+ setColumnWidths(storedColumnWidths || { name: '30%', value: 'auto' });
+ }, [tableId, storedColumnWidths]);🤖 Prompt for AI Agents |
||||||
|
|
||||||
| const [resizing, setResizing] = useState(null); | ||||||
| const [focusedNameIndex, setFocusedNameIndex] = useState(null); | ||||||
|
|
||||||
| const handleColumnWidthsChange = (id, widths) => { | ||||||
| dispatch(updateTableColumnWidths({ uid: activeTabUid, tableId: id, widths })); | ||||||
| }; | ||||||
|
|
||||||
| // Store column widths in ref for access in event handlers | ||||||
| const columnWidthsRef = useRef(columnWidths); | ||||||
| columnWidthsRef.current = columnWidths; | ||||||
|
|
||||||
| const handleResizeStart = useCallback((e, columnKey) => { | ||||||
| e.preventDefault(); | ||||||
| e.stopPropagation(); | ||||||
|
|
@@ -83,21 +108,24 @@ const EnvironmentVariablesTable = ({ | |||||
| const maxShrink = startWidth - MIN_COLUMN_WIDTH; | ||||||
| const clampedDiff = Math.max(-maxShrink, Math.min(maxGrow, diff)); | ||||||
|
|
||||||
| setColumnWidths({ | ||||||
| const newWidths = { | ||||||
| [columnKey]: `${startWidth + clampedDiff}px`, | ||||||
| [nextColumnKey]: `${nextColumnStartWidth - clampedDiff}px` | ||||||
| }); | ||||||
| }; | ||||||
| setColumnWidths(newWidths); | ||||||
| }; | ||||||
|
|
||||||
| const handleMouseUp = () => { | ||||||
| setResizing(null); | ||||||
| // Save to Redux after resize ends using ref for latest values | ||||||
| handleColumnWidthsChange(tableId, columnWidthsRef.current); | ||||||
| document.removeEventListener('mousemove', handleMouseMove); | ||||||
| document.removeEventListener('mouseup', handleMouseUp); | ||||||
| }; | ||||||
|
|
||||||
| document.addEventListener('mousemove', handleMouseMove); | ||||||
| document.addEventListener('mouseup', handleMouseUp); | ||||||
| }, []); | ||||||
| }, [handleColumnWidthsChange]); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing
- }, [handleColumnWidthsChange]);
+ }, [handleColumnWidthsChange, tableId]);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| const handleTotalHeightChanged = useCallback((h) => { | ||||||
| setTableHeight(h); | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either leave it uncontrolled and use refs or leave it controlled and pass it where necessary and handle when the prop isn't passed.