Skip to content

Commit 38d44a4

Browse files
LFDanLuLFDanLu
authored
Revert "@react-aria/overlays - Fix overlay position clamping (#3756)" (#3848)
This reverts commit e5d6994.
1 parent cb1fd97 commit 38d44a4

File tree

3 files changed

+29
-59
lines changed

3 files changed

+29
-59
lines changed

‎packages/@react-aria/overlays/src/calculatePosition.ts

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
*/
1212

1313
import {Axis, Placement, PlacementAxis, SizeAxis} from '@react-types/overlays';
14-
import {clamp} from '@react-aria/utils';
1514

1615
interface Position {
1716
top?: number,
@@ -203,16 +202,13 @@ function computePosition(
203202
// add the crossOffset from props
204203
position[crossAxis] += crossOffset;
205204

206-
// button bottom touching overlay bottom
207-
let bottomsTouchPosition = childOffset[crossAxis] + childOffset[crossSize] - overlaySize[crossSize];
208-
// button top touching overlay top
209-
let topsTouchPosition = childOffset[crossAxis];
205+
// this is button center position - the overlay size + half of the button to align bottom of overlay with button center
206+
let minViablePosition = childOffset[crossAxis] + (childOffset[crossSize] / 2) - overlaySize[crossSize];
207+
// this is button position of center, aligns top of overlay with button center
208+
let maxViablePosition = childOffset[crossAxis] + (childOffset[crossSize] / 2);
210209

211-
// we can't assume which one is bigger because button could be bigger than overlay and vice versa
212-
const minPosition = Math.min(bottomsTouchPosition, topsTouchPosition);
213-
const maxPosition = Math.max(bottomsTouchPosition, topsTouchPosition);
214-
215-
position[crossAxis] = clamp(position[crossAxis], minPosition, maxPosition);
210+
// clamp it into the range of the min/max positions
211+
position[crossAxis] = Math.min(Math.max(minViablePosition, position[crossAxis]), maxViablePosition);
216212

217213
// Floor these so the position isn't placed on a partial pixel, only whole pixels. Shouldn't matter if it was floored or ceiled, so chose one.
218214
if (placement === axis) {

‎packages/@react-aria/overlays/stories/UseOverlayPosition.stories.tsx

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,9 @@ import {useOverlayTriggerState} from '@react-stately/overlays';
2121
function Trigger(props: {
2222
withPortal: boolean,
2323
placement: Placement,
24-
maxHeight?: number,
25-
buttonWidth?: number
24+
maxHeight?: number
2625
}) {
27-
const {withPortal, placement, maxHeight, buttonWidth} = props;
26+
const {withPortal, placement, maxHeight} = props;
2827
const targetRef = React.useRef<HTMLButtonElement>(null);
2928
const overlayRef = React.useRef<HTMLDivElement>(null);
3029
const state = useOverlayTriggerState({
@@ -72,13 +71,7 @@ function Trigger(props: {
7271
}
7372
return (
7473
<div style={{position: 'relative', margin: 'auto'}}>
75-
<button
76-
ref={targetRef}
77-
{...triggerProps}
78-
style={{width: buttonWidth}}
79-
onClick={() => state.toggle()}>
80-
Trigger (open: {`${state.isOpen}`})
81-
</button>
74+
<button ref={targetRef} {...triggerProps} onClick={() => state.toggle()}>Trigger (open: {`${state.isOpen}`})</button>
8275
{state.isOpen && overlay}
8376
</div>
8477
);
@@ -89,6 +82,5 @@ storiesOf('UseOverlayPosition', module)
8982
.add('document.body container top', () => <Trigger withPortal placement="top" />)
9083
.add('positioned container bottom', () => <Trigger withPortal={false} placement="bottom" />)
9184
.add('positioned container top', () => <Trigger withPortal={false} placement="top" />)
92-
.add('buttonWidth=500 document.body bottom start', () => <Trigger withPortal buttonWidth={500} placement="bottom start" />)
9385
.add('maxHeight=200 container bottom', () => <Trigger withPortal maxHeight={200} placement="bottom" />)
9486
.add('maxHeight=200 container top', () => <Trigger withPortal maxHeight={200} placement="top" />);

‎packages/@react-aria/overlays/test/calculatePosition.test.js

Lines changed: 20 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -194,107 +194,95 @@ describe('calculatePosition', function () {
194194
noOffset: [50, 200, undefined, 100, 350],
195195
offsetBefore: [-200, 50, undefined, 0, 500],
196196
offsetAfter: [300, 350, undefined, 200, 200],
197-
crossAxisOffsetPositive: [50, 210, undefined, 90, 340],
198-
crossAxisOffsetNegative: [50, 190, undefined, 110, 360],
197+
crossAxisOffset: [50, 210, undefined, 90, 340],
199198
mainAxisOffset: [40, 200, undefined, 100, 350]
200199
},
201200
{
202201
placement: 'left top',
203202
noOffset: [50, 250, undefined, 50, 300],
204203
offsetBefore: [-200, 50, undefined, 0, 500],
205204
offsetAfter: [300, 350, undefined, 200, 200],
206-
crossAxisOffsetPositive: [50, 250, undefined, 50, 300],
207-
crossAxisOffsetNegative: [50, 240, undefined, 60, 310],
205+
crossAxisOffset: [50, 260, undefined, 40, 290],
208206
mainAxisOffset: [40, 250, undefined, 50, 300]
209207
},
210208
{
211209
placement: 'left bottom',
212210
noOffset: [50, 150, undefined, 150, 400],
213211
offsetBefore: [-200, 50, undefined, 0, 500],
214212
offsetAfter: [300, 350, undefined, 200, 200],
215-
crossAxisOffsetPositive: [50, 160, undefined, 140, 390],
216-
crossAxisOffsetNegative: [50, 150, undefined, 150, 400],
213+
crossAxisOffset: [50, 160, undefined, 140, 390],
217214
mainAxisOffset: [40, 150, undefined, 150, 400]
218215
},
219216
{
220217
placement: 'top',
221218
noOffset: [200, 50, 100, undefined, 200],
222219
offsetBefore: [50, -200, 0, undefined, 0],
223220
offsetAfter: [350, 300, 200, undefined, 450],
224-
crossAxisOffsetPositive: [210, 50, 90, undefined, 200],
225-
crossAxisOffsetNegative: [190, 50, 110, undefined, 200],
226-
mainAxisOffset: [200, 40, 100, undefined, 200]
221+
mainAxisOffset: [200, 40, 100, undefined, 200],
222+
crossAxisOffset: [210, 50, 90, undefined, 200]
227223
},
228224
{
229225
placement: 'top left',
230226
noOffset: [250, 50, 50, undefined, 200],
231227
offsetBefore: [50, -200, 0, undefined, 0],
232228
offsetAfter: [350, 300, 200, undefined, 450],
233-
crossAxisOffsetPositive: [250, 50, 50, undefined, 200],
234-
crossAxisOffsetNegative: [240, 50, 60, undefined, 200],
235-
mainAxisOffset: [250, 40, 50, undefined, 200]
229+
mainAxisOffset: [250, 40, 50, undefined, 200],
230+
crossAxisOffset: [260, 50, 40, undefined, 200]
236231
},
237232
{
238233
placement: 'top right',
239234
noOffset: [150, 50, 150, undefined, 200],
240235
offsetBefore: [50, -200, 0, undefined, 0],
241236
offsetAfter: [350, 300, 200, undefined, 450],
242-
crossAxisOffsetPositive: [160, 50, 140, undefined, 200],
243-
crossAxisOffsetNegative: [150, 50, 150, undefined, 200],
244-
mainAxisOffset: [150, 40, 150, undefined, 200]
237+
mainAxisOffset: [150, 40, 150, undefined, 200],
238+
crossAxisOffset: [160, 50, 140, undefined, 200]
245239
},
246240
{
247241
placement: 'bottom',
248242
noOffset: [200, 350, 100, undefined, 200],
249243
offsetBefore: [50, 100, 0, undefined, 450],
250244
offsetAfter: [350, 600, 200, undefined, 0],
251-
crossAxisOffsetPositive: [210, 350, 90, undefined, 200],
252-
crossAxisOffsetNegative: [190, 350, 110, undefined, 200],
253-
mainAxisOffset: [200, 360, 100, undefined, 190]
245+
mainAxisOffset: [200, 360, 100, undefined, 190],
246+
crossAxisOffset: [210, 350, 90, undefined, 200]
254247
},
255248
{
256249
placement: 'bottom left',
257250
noOffset: [250, 350, 50, undefined, 200],
258251
offsetBefore: [50, 100, 0, undefined, 450],
259252
offsetAfter: [350, 600, 200, undefined, 0],
260-
crossAxisOffsetPositive: [250, 350, 50, undefined, 200],
261-
crossAxisOffsetNegative: [240, 350, 60, undefined, 200],
262-
mainAxisOffset: [250, 360, 50, undefined, 190]
253+
mainAxisOffset: [250, 360, 50, undefined, 190],
254+
crossAxisOffset: [260, 350, 40, undefined, 200]
263255
},
264256
{
265257
placement: 'bottom right',
266258
noOffset: [150, 350, 150, undefined, 200],
267259
offsetBefore: [50, 100, 0, undefined, 450],
268260
offsetAfter: [350, 600, 200, undefined, 0],
269-
crossAxisOffsetPositive: [160, 350, 140, undefined, 200],
270-
crossAxisOffsetNegative: [150, 350, 150, undefined, 200],
271-
mainAxisOffset: [150, 360, 150, undefined, 190]
261+
mainAxisOffset: [150, 360, 150, undefined, 190],
262+
crossAxisOffset: [160, 350, 140, undefined, 200]
272263
},
273264
{
274265
placement: 'right',
275266
noOffset: [350, 200, undefined, 100, 350],
276267
offsetBefore: [100, 50, undefined, 0, 500],
277268
offsetAfter: [600, 350, undefined, 200, 200],
278-
crossAxisOffsetPositive: [350, 210, undefined, 90, 340],
279-
crossAxisOffsetNegative: [350, 190, undefined, 110, 360],
269+
crossAxisOffset: [350, 210, undefined, 90, 340],
280270
mainAxisOffset: [360, 200, undefined, 100, 350]
281271
},
282272
{
283273
placement: 'right top',
284274
noOffset: [350, 250, undefined, 50, 300],
285275
offsetBefore: [100, 50, undefined, 0, 500],
286276
offsetAfter: [600, 350, undefined, 200, 200],
287-
crossAxisOffsetPositive: [350, 250, undefined, 50, 300],
288-
crossAxisOffsetNegative: [350, 240, undefined, 60, 310],
277+
crossAxisOffset: [350, 260, undefined, 40, 290],
289278
mainAxisOffset: [360, 250, undefined, 50, 300]
290279
},
291280
{
292281
placement: 'right bottom',
293282
noOffset: [350, 150, undefined, 150, 400],
294283
offsetBefore: [100, 50, undefined, 0, 500],
295284
offsetAfter: [600, 350, undefined, 200, 200],
296-
crossAxisOffsetPositive: [350, 160, undefined, 140, 390],
297-
crossAxisOffsetNegative: [350, 150, undefined, 150, 400],
285+
crossAxisOffset: [350, 160, undefined, 140, 390],
298286
mainAxisOffset: [360, 150, undefined, 150, 400]
299287
}
300288
];
@@ -333,15 +321,9 @@ describe('calculatePosition', function () {
333321
);
334322
});
335323

336-
describe('cross axis offset positive', function () {
324+
describe('cross axis offset', function () {
337325
checkPosition(
338-
placement, getTargetDimension({left: 250, top: 250}), testCase.crossAxisOffsetPositive, 0, 10
339-
);
340-
});
341-
342-
describe('cross axis offset negative', function () {
343-
checkPosition(
344-
placement, getTargetDimension({left: 250, top: 250}), testCase.crossAxisOffsetNegative, 0, -10
326+
placement, getTargetDimension({left: 250, top: 250}), testCase.crossAxisOffset, 0, 10
345327
);
346328
});
347329
});

0 commit comments

Comments
 (0)