react-native-paper icon indicating copy to clipboard operation
react-native-paper copied to clipboard

Modal will appear once more when its mask is clicked.

Open Qyokizzzz opened this issue 2 years ago • 8 comments

Current behaviour

Modal appears once more before close when its mask is clicked.

Expected behaviour

Modal disappears directly when its mask is clicked.

How to reproduce?

import * as React from 'react';
import { Modal, Portal, Text, Button, PaperProvider } from 'react-native-paper';

const MyComponent = () => {
  const [visible, setVisible] = React.useState(false);

  const showModal = () => setVisible(true);
  const hideModal = () => setVisible(false);
  const containerStyle = {backgroundColor: 'white', padding: 20};

  return (
    <PaperProvider>
      <Portal>
        <Modal visible={visible} onDismiss={hideModal} contentContainerStyle={containerStyle}>
          <Text>Example Modal.  Click outside this area to dismiss.</Text>
        </Modal>
      </Portal>
      <Button style={{marginTop: 30}} onPress={showModal}>
        Show
      </Button>
    </PaperProvider>
  );
};

export default MyComponent;

Preview

https://github.com/callstack/react-native-paper/assets/42693551/eca1a8b4-eeaf-49d9-8c03-ecdc3f5757f9

My suggestion

suggestion

The current behavior is expected by the test case, which prevents my changes from passing all tests.

$ jest Modal
 FAIL  src/components/__tests__/Modal.test.tsx
  Modal
    √ animated value changes correctly (81 ms)
    by default
      √ should render passed children (255 ms)
      √ should render a backdrop in default theme's color (14 ms)
      √ should render a custom backdrop color if specified (8 ms)
      √ should receive appropriate top and bottom insets (11 ms)
    when open
      if closed via touching backdrop
        × will run the animation but not fade out (57 ms)
        √ should invoke the onDismiss function after the animation (16 ms)
      if closed via Android back button
        √ will run the animation but not fade out (27 ms)
        × should invoke the onDismiss function after the animation (32 ms)
    when open as non-dismissible modal
      if closed via touching backdrop
        √ will run the animation but not fade out (15 ms)
        √ should not invoke onDismiss (13 ms)
      if closed via Android back button
        √ will run the animation but not fade out (14 ms)
        √ should not invoke onDismiss (11 ms)
    when visible prop changes
      from false to true (closed to open)
        √ should run fade-in animation on opening (12 ms)
      from true to false (open to closed)
        √ should run fade-out animation on closing (16 ms)
        √ should not invoke onDismiss (12 ms)
        √ should close even if the dialog is not dismissible (12 ms)
    when visible prop changes again during the open/close animation
      while closing, back to true (visible)
        √ should keep the modal open (23 ms)
      while opening, back to false (hidden)
        √ should keep the modal closed (20 ms)

  ● Modal › when open › if closed via touching backdrop › will run the animation but not fade out

    expect(element).toHaveStyle()

    - Expected

    - opacity: 1;
    + opacity: 0;

      123 |         });
      124 |
    > 125 |         expect(getByTestId('modal-backdrop')).toHaveStyle({
          |                                               ^
      126 |           opacity: 1,
      127 |         });
      128 |

      at Object.toHaveStyle (src/components/__tests__/Modal.test.tsx:125:47)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:22:9)
      at node_modules/@babel/runtime/helpers/asyncToGenerator.js:27:7
      at Object.<anonymous> (node_modules/@babel/runtime/helpers/asyncToGenerator.js:19:12)

  ● Modal › when open › if closed via Android back button › should invoke the onDismiss function after the animation

    expect(jest.fn()).toHaveBeenCalledTimes(expected)

    Expected number of calls: 1
    Received number of calls: 0

      210 |         });
      211 |
    > 212 |         expect(onDismiss).toHaveBeenCalledTimes(1);
          |                           ^
      213 |       });
      214 |     });
      215 |   });

      at Object.toHaveBeenCalledTimes (src/components/__tests__/Modal.test.tsx:212:27)

Test Suites: 1 failed, 1 total
Tests:       2 failed, 17 passed, 19 total
Snapshots:   0 total
Time:        1.789 s, estimated 2 s

Your Environment

software version
android 12
react-native 0.72.5
react-native-paper 5.10.4
node 18.12.1
yarn 1.22.19

Qyokizzzz avatar Oct 19 '23 16:10 Qyokizzzz

Is there any solution for this?

CXiann avatar Oct 31 '23 14:10 CXiann

Is there any solution for this?

Modify the Modal source code, please refer to 'My suggestion' for details.

Qyokizzzz avatar Oct 31 '23 14:10 Qyokizzzz

Hey @CXiann, I was trying to reproduce the issue, however without the success. Could you please attach the small github repo presenting the problem?

I was trying on:

react-native-paper: 5.10.4 and 5.11.1 react-native: 0.72.5 android: 12 new architecture: enabled and disabled

lukewalczak avatar Nov 18 '23 10:11 lukewalczak

im changed newArchEnabled to false on android/gradle.properties. and it's worked on me.

raihanadfal avatar Jan 05 '24 10:01 raihanadfal

I have the same issue. The Modal "blinks" only when you're clicking outside the modal. But when I put a button inside a Modal and press it for close – it closes without a blink. It is not reproducing on iOS.

react-native-paper: 5.10.3 react-native: 0.72.4 New architecture enabled. Android 10.

dabakovich avatar Jan 18 '24 16:01 dabakovich

I have the same issue. I think because the hideModal was called twice. And if your tested in a good device with small code, the delay between 2 animations is too small for recognizing.

Try to test with higher duration.

image

nyagami avatar Feb 15 '24 08:02 nyagami

I tried dismissable={false} on Dialog component and its work like this <Dialog visible={visible} dismissable={false}> </Dialog>

ErMapsh avatar Feb 27 '24 05:02 ErMapsh