fix: Hide popup no need re-align (#203)

react-component · Oct 12, 2020 · e57f149 · e57f149
1 parent b8b8cb2
commit e57f149
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 13 deletions.
diff --git a/src/index.tsx b/src/index.tsx
@@ -583,7 +583,7 @@ export function generateTrigger(
      * @param popupVisible    Show or not the popup element
      * @param event           SyntheticEvent, used for `pointAlign`
      */
-    setPopupVisible(popupVisible, event?) {
+    setPopupVisible(popupVisible: boolean, event?: { pageX: number, pageY: number }) {
       const { alignPoint } = this.props;
       const { popupVisible: prevPopupVisible } = this.state;
 
@@ -597,7 +597,7 @@ export function generateTrigger(
       }
 
       // Always record the point position since mouseEnterDelay will delay the show
-      if (alignPoint && event) {
+      if (alignPoint && event && popupVisible) {
         this.setPoint(event);
       }
     }

diff --git a/tests/point.test.jsx b/tests/point.test.jsx
@@ -16,11 +16,16 @@ describe('Trigger.Point', () => {
   });
 
   class Demo extends React.Component {
-    popup = <div className="point-popup">POPUP</div>;
+    popup = (<div className="point-popup">POPUP</div>);
 
     render() {
       return (
-        <Trigger popup={this.popup} popupAlign={{ points: ['tl'] }} alignPoint {...this.props}>
+        <Trigger
+          popup={this.popup}
+          popupAlign={{ points: ['tl'] }}
+          alignPoint
+          {...this.props}
+        >
           <div className="point-region" />
         </Trigger>
       );
@@ -36,7 +41,9 @@ describe('Trigger.Point', () => {
       .first()
       .getDOMNode();
 
-    expect(popup.style).toEqual(expect.objectContaining({ left: '-989px', top: '-979px' }));
+    expect(popup.style).toEqual(
+      expect.objectContaining({ left: '-989px', top: '-979px' }),
+    );
   });
 
   it('hover', () => {
@@ -48,33 +55,59 @@ describe('Trigger.Point', () => {
       .first()
       .getDOMNode();
 
-    expect(popup.style).toEqual(expect.objectContaining({ left: '-989px', top: '-979px' }));
+    expect(popup.style).toEqual(
+      expect.objectContaining({ left: '-989px', top: '-979px' }),
+    );
   });
 
   describe('contextMenu', () => {
     it('basic', () => {
-      const wrapper = mount(<Demo action={['contextMenu']} />);
+      const wrapper = mount(
+        <Demo action={['contextMenu']} hideAction={['click']} />,
+      );
       wrapper.trigger('contextMenu', { pageX: 10, pageY: 20 });
 
       const popup = wrapper
         .find('.rc-trigger-popup')
         .first()
         .getDOMNode();
 
-      expect(popup.style).toEqual(expect.objectContaining({ left: '-989px', top: '-979px' }));
+      expect(popup.style).toEqual(
+        expect.objectContaining({ left: '-989px', top: '-979px' }),
+      );
+
+      // Not trigger point update when close
+      const clickEvent = {};
+      const pagePropDefine = {
+        get: () => {
+          throw new Error('should not read when close');
+        },
+      };
+      Object.defineProperties(clickEvent, {
+        pageX: pagePropDefine,
+        pageY: pagePropDefine,
+      });
+      wrapper
+        .find('Trigger')
+        .instance()
+        .onClick(clickEvent);
     });
 
     // https://github.com/ant-design/ant-design/issues/17043
     it('not prevent default', done => {
-      const wrapper = mount(<Demo showAction={['contextMenu']} hideAction={['click']} />);
+      const wrapper = mount(
+        <Demo showAction={['contextMenu']} hideAction={['click']} />,
+      );
       wrapper.trigger('contextMenu', { pageX: 10, pageY: 20 });
 
       const popup = wrapper
         .find('.rc-trigger-popup')
         .first()
         .getDOMNode();
 
-      expect(popup.style).toEqual(expect.objectContaining({ left: '-989px', top: '-979px' }));
+      expect(popup.style).toEqual(
+        expect.objectContaining({ left: '-989px', top: '-979px' }),
+      );
 
       // Click to close
       wrapper.trigger('click', {
@@ -91,7 +124,11 @@ describe('Trigger.Point', () => {
     function testPlacement(name, builtinPlacements, afterAll) {
       it(name, () => {
         const wrapper = mount(
-          <Demo action={['click']} builtinPlacements={builtinPlacements} popupPlacement="right" />,
+          <Demo
+            action={['click']}
+            builtinPlacements={builtinPlacements}
+            popupPlacement="right"
+          />,
         );
         wrapper.trigger('click', { pageX: 10, pageY: 20 });
 
@@ -100,7 +137,9 @@ describe('Trigger.Point', () => {
           .first()
           .getDOMNode();
 
-        expect(popup.style).toEqual(expect.objectContaining({ left: '-989px', top: '-979px' }));
+        expect(popup.style).toEqual(
+          expect.objectContaining({ left: '-989px', top: '-979px' }),
+        );
 
         if (afterAll) {
           afterAll(wrapper);
@@ -124,7 +163,9 @@ describe('Trigger.Point', () => {
       },
       wrapper => {
         expect(
-          wrapper.find('div.rc-trigger-popup').hasClass('rc-trigger-popup-placement-left'),
+          wrapper
+            .find('div.rc-trigger-popup')
+            .hasClass('rc-trigger-popup-placement-left'),
         ).toBeTruthy();
       },
     );