Skip to content

Commit f33288c

Browse files
authored
fix: Revert displaying popovers as modals on mobile for now (#7392)
* Revert displaying popovers as modals on mobile for now * Improve story example * Add padding between fullscreen dialog content and buttons on small screens * Remove mobileType for now
1 parent f2d996d commit f33288c

File tree

3 files changed

+34
-29
lines changed

3 files changed

+34
-29
lines changed

packages/@react-spectrum/s2/src/FullscreenDialog.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ export const dialogInner = style({
7272
'header',
7373
'.',
7474
'content',
75+
'.',
7576
'buttons'
7677
],
7778
sm: [
@@ -90,6 +91,7 @@ export const dialogInner = style({
9091
'auto',
9192
24,
9293
'1fr',
94+
24,
9395
'auto'
9496
],
9597
sm: [

packages/@react-spectrum/s2/src/Popover.tsx

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,19 @@ import {
1616
composeRenderProps,
1717
Dialog,
1818
DialogProps,
19-
ModalRenderProps,
2019
OverlayArrow,
2120
OverlayTriggerStateContext,
2221
useLocale
2322
} from 'react-aria-components';
2423
import {colorScheme, getAllowedOverrides, StyleProps, UnsafeStyles} from './style-utils' with {type: 'macro'};
2524
import {ColorSchemeContext} from './Provider';
26-
import {DismissButton} from 'react-aria';
2725
import {DOMRef} from '@react-types/shared';
2826
import {forwardRef, MutableRefObject, useCallback, useContext} from 'react';
2927
import {keyframes} from '../style/style-macro' with {type: 'macro'};
3028
import {mergeStyles} from '../style/runtime';
31-
import {Modal} from './Modal';
3229
import {style} from '../style' with {type: 'macro'};
3330
import {StyleString} from '../style/types' with {type: 'macro'};
34-
import {useDOMRef, useIsMobileDevice} from '@react-spectrum/utils';
31+
import {useDOMRef} from '@react-spectrum/utils';
3532

3633
export interface PopoverProps extends UnsafeStyles, Omit<AriaPopoverProps, 'arrowSize' | 'isNonModal' | 'arrowBoundaryOffset' | 'isKeyboardDismissDisabled' | 'shouldCloseOnInteractOutside' | 'shouldUpdatePosition'> {
3734
styles?: StyleString,
@@ -44,9 +41,9 @@ export interface PopoverProps extends UnsafeStyles, Omit<AriaPopoverProps, 'arro
4441
/**
4542
* The size of the Popover. If not specified, the popover fits its contents.
4643
*/
47-
size?: 'S' | 'M' | 'L',
44+
size?: 'S' | 'M' | 'L'
4845
/** The type of overlay that should be rendered when on a mobile device. */
49-
mobileType?: 'modal' | 'fullscreen' | 'fullscreenTakeover' // TODO: add tray back in
46+
// mobileType?: 'modal' | 'fullscreen' | 'fullscreenTakeover' // TODO: add tray back in
5047
}
5148

5249
const fadeKeyframes = keyframes(`
@@ -126,6 +123,9 @@ let popover = style({
126123
L: 576
127124
}
128125
},
126+
// Don't be larger than full screen minus 2 * containerPadding
127+
maxWidth: '[calc(100vw - 24px)]',
128+
boxSizing: 'border-box',
129129
translateY: {
130130
placement: {
131131
bottom: {
@@ -221,9 +221,7 @@ function PopoverBase(props: PopoverProps, ref: DOMRef<HTMLDivElement>) {
221221
UNSAFE_className = '',
222222
UNSAFE_style,
223223
styles,
224-
size,
225-
children,
226-
trigger = null
224+
size
227225
} = props;
228226
let domRef = useDOMRef(ref);
229227
let colorScheme = useContext(ColorSchemeContext);
@@ -239,24 +237,25 @@ function PopoverBase(props: PopoverProps, ref: DOMRef<HTMLDivElement>) {
239237
}, [locale, direction, domRef]);
240238

241239
// On small devices, show a modal (or eventually a tray) instead of a popover.
242-
let isMobile = useIsMobileDevice();
243-
if (isMobile && process.env.NODE_ENV !== 'test') {
244-
let mappedChildren = typeof children === 'function'
245-
? (renderProps: ModalRenderProps) => children({...renderProps, defaultChildren: null, trigger, placement: 'bottom'})
246-
: children;
240+
// TODO: reverted this until we have trays.
241+
// let isMobile = useIsMobileDevice();
242+
// if (isMobile && process.env.NODE_ENV !== 'test') {
243+
// let mappedChildren = typeof children === 'function'
244+
// ? (renderProps: ModalRenderProps) => children({...renderProps, defaultChildren: null, trigger, placement: 'bottom'})
245+
// : children;
247246

248-
return (
249-
<Modal size={size} isDismissable>
250-
{composeRenderProps(mappedChildren, (children, {state}) => (
251-
<>
252-
{children}
253-
{/* Add additional dismiss button at the end to match popovers. */}
254-
<DismissButton onDismiss={state.close} />
255-
</>
256-
))}
257-
</Modal>
258-
);
259-
}
247+
// return (
248+
// <Modal size={size} isDismissable>
249+
// {composeRenderProps(mappedChildren, (children, {state}) => (
250+
// <>
251+
// {children}
252+
// {/* Add additional dismiss button at the end to match popovers. */}
253+
// <DismissButton onDismiss={state.close} />
254+
// </>
255+
// ))}
256+
// </Modal>
257+
// );
258+
// }
260259

261260
// TODO: this still isn't the final popover 'tip', copying various ones out of the designs files yields different results
262261
// containerPadding not working as expected
@@ -289,7 +288,7 @@ function PopoverBase(props: PopoverProps, ref: DOMRef<HTMLDivElement>) {
289288
let _PopoverBase = forwardRef(PopoverBase);
290289
export {_PopoverBase as PopoverBase};
291290

292-
export interface PopoverDialogProps extends Pick<PopoverProps, 'size' | 'hideArrow' | 'mobileType' | 'placement' | 'shouldFlip' | 'containerPadding' | 'offset' | 'crossOffset'>, Omit<DialogProps, 'className' | 'style'>, StyleProps {}
291+
export interface PopoverDialogProps extends Pick<PopoverProps, 'size' | 'hideArrow'| 'placement' | 'shouldFlip' | 'containerPadding' | 'offset' | 'crossOffset'>, Omit<DialogProps, 'className' | 'style'>, StyleProps {}
293292

294293
const dialogStyle = style({
295294
padding: 8,

packages/@react-spectrum/s2/stories/Popover.stories.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* governing permissions and limitations under the License.
1111
*/
1212

13-
import {ActionButton, Avatar, Button, Card, CardPreview, Content, DialogTrigger, Divider, DropZone, Form, Image, Menu, MenuItem, MenuSection, Popover, SearchField, SubmenuTrigger, Switch, Tab, TabList, TabPanel, Tabs, Text, TextField} from '../src';
13+
import {ActionButton, Avatar, Button, Card, CardPreview, Content, DialogTrigger, Divider, Form, Image, Menu, MenuItem, MenuSection, Popover, SearchField, SubmenuTrigger, Switch, Tab, TabList, TabPanel, Tabs, Text, TextField} from '../src';
1414
import Cloud from '../s2wf-icons/S2_Icon_Cloud_20_N.svg';
1515
import Education from '../s2wf-icons/S2_Icon_Education_20_N.svg';
1616
import File from '../s2wf-icons/S2_Icon_File_20_N.svg';
@@ -97,7 +97,6 @@ export const HelpCenter = (args: any) => (
9797
<Form>
9898
<TextField label="Subject" />
9999
<TextField label="Description" isRequired />
100-
<DropZone />
101100
<Switch>Adobe can contact me for further questions concerning this feedback</Switch>
102101
<Button styles={style({marginStart: 'auto'})}>Submit</Button>
103102
</Form>
@@ -159,3 +158,8 @@ export const AccountMenu = (args: any) => (
159158
</Popover>
160159
</DialogTrigger>
161160
);
161+
162+
AccountMenu.argTypes = {
163+
hideArrow: {table: {disable: true}},
164+
placement: {table: {disable: true}}
165+
};

0 commit comments

Comments
 (0)