Skip to content

Commit 4d300d1

Browse files
ilyabobdjulbic
ilyabo
authored and
bdjulbic
committed
fix: Clamp legend height if it exceeds available space (keplergl#3276)
* fix: Clamp legend height if it exceeds available space Signed-off-by: Ilya Boyandin <ilyabo@gmail.com> * simplify Signed-off-by: Ilya Boyandin <ilyabo@gmail.com> * fixes Signed-off-by: Ilya Boyandin <ilyabo@gmail.com> * PR feedback Signed-off-by: Ilya Boyandin <ilyabo@gmail.com> --------- Signed-off-by: Ilya Boyandin <ilyabo@gmail.com> Signed-off-by: bdjulbic <bdjulbic@foursquare.com>
1 parent 9d82555 commit 4d300d1

3 files changed

Lines changed: 131 additions & 15 deletions

File tree

‎src/components/src/hooks/use-legend-position.ts‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,15 @@ type Params = {
1111
settings?: MapLegendControlSettings;
1212
onChangeSettings: (settings: Partial<MapLegendControlSettings>) => void;
1313
theme: Record<string, any>;
14+
mapHeight?: number;
15+
mapWidth?: number;
1416
};
1517

1618
type ReturnType = {
1719
positionStyles: Record<string, unknown>;
1820
updatePosition: () => void;
1921
contentHeight: number;
22+
maxContentHeight?: number;
2023
startResize: () => void;
2124
resize: (deltaY: number) => void;
2225
};
@@ -87,14 +90,23 @@ export default function useLegendPosition({
8790
isSidePanelShown,
8891
settings,
8992
onChangeSettings,
90-
theme
93+
theme,
94+
mapHeight,
95+
mapWidth
9196
}: Params): ReturnType {
9297
const pos = settings?.position ?? DEFAULT_POSITION;
9398
const contentHeight = settings?.contentHeight ?? -1;
9499
const positionStyles = useMemo(() => ({[pos.anchorX]: pos.x, [pos.anchorY]: pos.y}), [pos]);
95100
const startHeightRef = useRef(0);
96101
const sidePanelWidth = theme.sidePanel?.width || 0;
97102

103+
// Calculate dynamic max content height based on map root dimensions
104+
const maxContentHeight = useMemo(() => {
105+
if (!mapHeight) return undefined;
106+
// Available height minus margins and header
107+
return mapHeight - MARGIN.top - MARGIN.bottom - MAP_CONTROL_HEADER_FULL_HEIGHT;
108+
}, [mapHeight]);
109+
98110
const calcPosition = useCalcLegendPosition({
99111
legendContentRef,
100112
isSidePanelShown,
@@ -119,8 +131,14 @@ export default function useLegendPosition({
119131
if (root instanceof HTMLElement && legendContent) {
120132
const mapRootBounds = root.getBoundingClientRect();
121133
const legendRect = legendContent.getBoundingClientRect();
134+
const remainingHeight =
135+
mapRootBounds.bottom - (legendRect.top + MAP_CONTROL_HEADER_FULL_HEIGHT + MARGIN.bottom);
136+
// Use maxContentHeight if available, otherwise fall back to viewport-based calculation
137+
const maxHeight = maxContentHeight
138+
? Math.min(maxContentHeight, remainingHeight)
139+
: remainingHeight;
122140
const nextHeight = Math.min(
123-
mapRootBounds.bottom - (legendRect.top + MAP_CONTROL_HEADER_FULL_HEIGHT + MARGIN.bottom),
141+
maxHeight,
124142
Math.max(MIN_CONTENT_HEIGHT, startHeightRef.current + deltaY)
125143
);
126144
onChangeSettings({contentHeight: nextHeight});
@@ -130,7 +148,7 @@ export default function useLegendPosition({
130148
}
131149
},
132150
// eslint-disable-next-line react-hooks/exhaustive-deps
133-
[contentHeight, pos, onChangeSettings]
151+
[contentHeight, pos, onChangeSettings, maxContentHeight]
134152
);
135153

136154
// Shift when side panel is shown/hidden
@@ -151,5 +169,13 @@ export default function useLegendPosition({
151169
}
152170
}, [isSidePanelShown, onChangeSettings, sidePanelWidth]);
153171

154-
return {positionStyles, updatePosition, contentHeight, startResize, resize};
172+
// Clamp contentHeight when map resizes to ensure legend stays within available space
173+
useEffect(() => {
174+
if (!mapWidth || !mapHeight || !legendContentRef.current) return;
175+
if (maxContentHeight && contentHeight > 0 && contentHeight > maxContentHeight) {
176+
onChangeSettings({contentHeight: maxContentHeight});
177+
}
178+
}, [mapWidth, mapHeight, contentHeight, onChangeSettings, maxContentHeight, legendContentRef]);
179+
180+
return {positionStyles, updatePosition, contentHeight, maxContentHeight, startResize, resize};
155181
}

‎src/components/src/map/map-legend-panel.tsx‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@ import {restrictToWindowEdges} from '@dnd-kit/modifiers';
3737
const DRAG_RESIZE_ID = 'map-legend-resize';
3838
const DRAG_MOVE_ID = 'map-legend-move';
3939

40-
const StyledDraggableLegendContent = styled.div<{contentHeight?: number}>`
40+
const StyledDraggableLegendContent = styled.div<{
41+
contentHeight?: number;
42+
maxContentHeight?: number;
43+
}>`
4144
position: absolute;
4245
outline: none;
4346
transition: border-color 0.2s ease-in-out;
@@ -67,7 +70,8 @@ const StyledDraggableLegendContent = styled.div<{contentHeight?: number}>`
6770
border-color: ${props => props.theme.activeColor};
6871
}
6972
.map-control__panel-content {
70-
max-height: calc(100vh - 100px);
73+
max-height: ${props =>
74+
props.maxContentHeight ? `${props.maxContentHeight}px` : 'calc(100vh - 100px)'};
7175
${props => (props.contentHeight ? `height: ${props.contentHeight}px;` : '')};
7276
}
7377
border-radius: 4px;
@@ -141,12 +145,13 @@ export type MapLegendPanelFactoryDeps = [
141145

142146
type DraggableLegendContentProps = {
143147
contentHeight?: number;
148+
maxContentHeight?: number;
144149
positionStyles: Record<string, unknown>;
145150
children: React.ReactNode;
146151
};
147152

148153
const DraggableLegendContent = forwardRef((props: DraggableLegendContentProps, ref) => {
149-
const {positionStyles, children} = props;
154+
const {positionStyles, children, contentHeight, maxContentHeight} = props;
150155
const draggableMove = useDraggable({id: DRAG_MOVE_ID});
151156
const draggableResize = useDraggable({id: DRAG_RESIZE_ID});
152157
const refs = useMergeRefs([draggableMove.setNodeRef, ref]);
@@ -156,7 +161,8 @@ const DraggableLegendContent = forwardRef((props: DraggableLegendContentProps, r
156161
ref={refs}
157162
className={classnames('draggable-legend', {'is-dragging': isDragging})}
158163
style={{...positionStyles, transform: CSS.Translate.toString(draggableMove.transform)}}
159-
contentHeight={props.contentHeight}
164+
contentHeight={contentHeight}
165+
maxContentHeight={maxContentHeight}
160166
{...draggableMove.attributes}
161167
>
162168
{children}
@@ -179,6 +185,7 @@ type DraggableLegendProps = PropsWithChildren<{
179185
isSidePanelShown: boolean;
180186
mapControls: MapControls;
181187
setMapControlSettings: typeof setMapControlSettings;
188+
mapState?: MapState;
182189
}>;
183190

184191
const DraggableLegend = withTheme(
@@ -187,6 +194,7 @@ const DraggableLegend = withTheme(
187194
children,
188195
mapControls,
189196
setMapControlSettings,
197+
mapState,
190198
theme
191199
}: DraggableLegendProps & {theme: any}) => {
192200
const settings = mapControls?.mapLegend?.settings;
@@ -196,13 +204,16 @@ const DraggableLegend = withTheme(
196204
newSettings => setMapControlSettings('mapLegend', newSettings),
197205
[setMapControlSettings]
198206
);
199-
const {positionStyles, updatePosition, startResize, resize, contentHeight} = useLegendPosition({
200-
legendContentRef,
201-
isSidePanelShown,
202-
theme,
203-
settings,
204-
onChangeSettings
205-
});
207+
const {positionStyles, updatePosition, startResize, resize, contentHeight, maxContentHeight} =
208+
useLegendPosition({
209+
legendContentRef,
210+
isSidePanelShown,
211+
theme,
212+
settings,
213+
onChangeSettings,
214+
mapHeight: mapState?.height,
215+
mapWidth: mapState?.width
216+
});
206217

207218
const handleDragStart = useCallback(
208219
event => {
@@ -239,6 +250,7 @@ const DraggableLegend = withTheme(
239250
ref={legendContentRef}
240251
positionStyles={positionStyles}
241252
contentHeight={contentHeight}
253+
maxContentHeight={maxContentHeight}
242254
>
243255
{children}
244256
</DraggableLegendContent>
@@ -413,6 +425,7 @@ const MapLegendPanelComponent = ({
413425
isSidePanelShown={isSidePanelShown}
414426
mapControls={mapControls}
415427
setMapControlSettings={setMapControlSettings}
428+
mapState={mapState}
416429
>
417430
{legendPanel}
418431
</DraggableLegend>,

‎test/browser/components/hooks/use-legend-position.spec.js‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,81 @@ describe('useLegendPosition', () => {
5656
);
5757
expect(positionStyles).toEqual({left: 100, top: 200});
5858
});
59+
60+
test('should calculate maxContentHeight from mapWidth and mapHeight', () => {
61+
const {
62+
result: {
63+
current: {maxContentHeight}
64+
}
65+
} = renderHook(() =>
66+
useLegendPosition({
67+
legendContentRef: {current: document.querySelector('#map-legend-content')},
68+
isSidePanelShown: false,
69+
settings: {},
70+
onChangeSettings: jest.fn(),
71+
theme: THEME,
72+
mapHeight: 600,
73+
mapWidth: 800
74+
})
75+
);
76+
// maxContentHeight = height - MARGIN.top - MARGIN.bottom - MAP_CONTROL_HEADER_FULL_HEIGHT
77+
// = 600 - 10 - 30 - 34 = 526
78+
expect(maxContentHeight).toBe(526);
79+
});
80+
81+
test('should return undefined maxContentHeight when mapWidth and mapHeight not provided', () => {
82+
const {
83+
result: {
84+
current: {maxContentHeight}
85+
}
86+
} = renderHook(() =>
87+
useLegendPosition({
88+
legendContentRef: {current: document.querySelector('#map-legend-content')},
89+
isSidePanelShown: false,
90+
settings: {},
91+
onChangeSettings: jest.fn(),
92+
theme: THEME
93+
})
94+
);
95+
expect(maxContentHeight).toBeUndefined();
96+
});
97+
98+
test('should clamp contentHeight when it exceeds maxContentHeight', () => {
99+
const onChangeSettings = jest.fn();
100+
const legendContent = document.querySelector('#map-legend-content');
101+
legendContent.getBoundingClientRect = jest.fn(() => ({
102+
width: 200,
103+
height: 300,
104+
top: 0,
105+
left: 0,
106+
bottom: 300,
107+
right: 200
108+
}));
109+
110+
renderHook(() =>
111+
useLegendPosition({
112+
legendContentRef: {current: legendContent},
113+
isSidePanelShown: false,
114+
settings: {
115+
position: {x: 100, y: 100, anchorX: 'left', anchorY: 'top'},
116+
contentHeight: 1000 // Exceeds maxContentHeight
117+
},
118+
onChangeSettings,
119+
theme: THEME,
120+
mapHeight: 600,
121+
mapWidth: 800
122+
})
123+
);
124+
125+
// Should clamp contentHeight to maxContentHeight (526)
126+
expect(onChangeSettings).toHaveBeenCalled();
127+
const callsWithContentHeight = onChangeSettings.mock.calls.filter(
128+
call => call[0].contentHeight !== undefined
129+
);
130+
if (callsWithContentHeight.length > 0) {
131+
expect(
132+
callsWithContentHeight[callsWithContentHeight.length - 1][0].contentHeight
133+
).toBeLessThanOrEqual(526);
134+
}
135+
});
59136
});

0 commit comments

Comments
 (0)