From 2ba123ed93d81a81463437bf068ec7e56c38bfe7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E9=99=88=E5=98=89=E6=B6=B5?= <chenjiahan@meituan.com>
Date: Thu, 17 Aug 2017 14:08:21 +0800
Subject: [PATCH] actionsheet code review

---
 packages/actionsheet/src/actionsheet.vue | 78 ++++++++----------------
 packages/vant-css/src/actionsheet.css    | 39 +++++-------
 test/unit/specs/actionsheet.spec.js      | 29 +++++++--
 test/unit/utils.js                       | 46 ++++++++++++++
 4 files changed, 111 insertions(+), 81 deletions(-)
 create mode 100644 test/unit/utils.js

diff --git a/packages/actionsheet/src/actionsheet.vue b/packages/actionsheet/src/actionsheet.vue
index 7d6e4398d..6a3a9c170 100644
--- a/packages/actionsheet/src/actionsheet.vue
+++ b/packages/actionsheet/src/actionsheet.vue
@@ -1,40 +1,35 @@
 <template>
-  <transition name="actionsheet-float">
-    <div class="van-actionsheet" :class="{ 'van-actionsheet--withtitle': title }" v-show="currentValue">
+  <transition name="van-actionsheet-float">
+    <div :class="['van-actionsheet', { 'van-actionsheet--withtitle': title }]" v-show="value">
       <div class="van-actionsheet__header" v-if="title">
-        <h3 v-text="title"></h3>
-        <van-icon name="close" @click.stop="currentValue = false"></van-icon>
+        <h3 v-text="title" />
+        <van-icon name="close" @click.stop="$emit('input', false)" />
+      </div>
+      <ul v-if="!title" class="van-actionsheet__list">
+        <li
+          v-for="(item, index) in actions"
+          :key="index"
+          :class="['van-actionsheet__item', item.className, { 'van-actionsheet__item--loading': item.loading }]"
+          @click.stop="handleItemClick(item)">
+          <template v-if="!item.loading">
+            <span class="van-actionsheet__name">{{ item.name }}</span>
+            <span class="van-actionsheet__subname" v-if="item.subname">{{ item.subname }}</span>
+          </template>
+          <van-loading v-else class="van-actionsheet__loading" type="circle" color="black" />
+        </li>
+      </ul>
+      <div class="van-actionsheet__item van-actionsheet__cancel" @click.stop="$emit('input', false)" v-if="cancelText">{{ cancelText }}</div>
+      <div class="van-actionsheet__content" v-else>
+        <slot></slot>
       </div>
-      <template v-if="!title">
-        <ul class="van-actionsheet__list">
-          <li
-            v-for="(item, index) in actions"
-            :key="index"
-            class="van-actionsheet__item"
-            :class="[item.className, item.loading ? 'van-actionsheet__item--loading' : '']"
-            @click.stop="handleItemClick(item)">
-            <template v-if="!item.loading">
-              <span class="van-actionsheet__name">{{ item.name }}</span>
-              <span class="van-actionsheet__subname" v-if="item.subname">{{ item.subname }}</span>
-            </template>
-            <van-loading v-else class="van-actionsheet__loading" type="circle" color="black" />
-          </li>
-        </ul>
-        <a class="van-actionsheet__button" @click.stop="currentValue = false" v-if="cancelText">{{ cancelText }}</a>
-      </template>
-      <template v-else>
-        <div class="van-actionsheet__content">
-          <slot></slot>
-        </div>
-      </template>
     </div>
   </transition>
 </template>
 
 <script>
 import Popup from '../../mixins/popup';
-import VanLoading from '../../loading';
 import VanIcon from '../../icon';
+import VanLoading from '../../loading';
 
 export default {
   name: 'van-actionsheet',
@@ -42,12 +37,12 @@ export default {
   mixins: [Popup],
 
   components: {
-    VanLoading,
-    VanIcon
+    [VanIcon.name]: VanIcon,
+    [VanLoading.name]: VanLoading
   },
 
   props: {
-    value: {},
+    value: Boolean,
     actions: {
       type: Array,
       default: () => []
@@ -64,32 +59,13 @@ export default {
     }
   },
 
-  data() {
-    return {
-      currentValue: this.value
-    };
-  },
-
-  watch: {
-    currentValue(val) {
-      this.$emit('input', val);
-    },
-
-    value(val) {
-      this.currentValue = val;
-    }
-  },
-
   mounted() {
-    if (this.value) {
-      this.currentValue = true;
-      this.open();
-    }
+    this.value && this.open();
   },
 
   methods: {
     handleItemClick(item) {
-      if (item.callback && typeof item.callback === 'function') {
+      if (typeof item.callback === 'function') {
         item.callback(item);
       }
     }
diff --git a/packages/vant-css/src/actionsheet.css b/packages/vant-css/src/actionsheet.css
index 2f37bbfd9..ffae808ff 100644
--- a/packages/vant-css/src/actionsheet.css
+++ b/packages/vant-css/src/actionsheet.css
@@ -4,26 +4,23 @@
 
 .van-actionsheet {
   position: fixed;
-  width: 100%;
-  top: auto;
+  left: 0;
+  right: 0;
   bottom: 0;
-  right: auto;
-  left: 50%;
-  transform: translate3d(-50%, 0, 0);
-  backface-visibility: hidden;
+  color: $c-black;
   transition: .2s ease-out;
   background-color: #e0e0e0;
+  backface-visibility: hidden;
 
   &--withtitle {
     background-color: $c-white;
   }
 
   &__item {
-    line-height: 50px;
     height: 50px;
-    text-align: center;
-    color: $c-black;
+    line-height: 50px;
     font-size: 16px;
+    text-align: center;
     position: relative;
     background-color: $c-white;
 
@@ -33,28 +30,20 @@
   }
 
   &__subname {
-    color: $c-gray-darker;
     font-size: 12px;
+    color: $c-gray-darker;
   }
 
   &__loading {
-    margin: 0 auto;
     display: inline-block;
   }
 
-  &__button {
-    display: block;
+  &__cancel {
     margin-top: 5px;
-    line-height: 50px;
-    color: $c-black;
-    font-size: 16px;
-    text-align: center;
-    background-color: $c-white;
   }
 
   &__header {
     line-height: 44px;
-    color: $c-black;
     text-align: center;
     position: relative;
 
@@ -63,16 +52,16 @@
     }
 
     .van-icon-close {
+      top: 11px;
+      right: 15px;
       position: absolute;
       font-size: 22px;
       line-height: 22px;
-      top: 11px;
-      right: 15px;
     }
   }
-}
 
-.actionsheet-float-enter,
-.actionsheet-float-leave-active {
-  transform: translate3d(-50%, 100%, 0);
+  &-float-enter,
+  &-float-leave-active {
+    transform: translate3d(0, 100%, 0);
+  }
 }
diff --git a/test/unit/specs/actionsheet.spec.js b/test/unit/specs/actionsheet.spec.js
index 8db863831..cd6a1d942 100644
--- a/test/unit/specs/actionsheet.spec.js
+++ b/test/unit/specs/actionsheet.spec.js
@@ -1,5 +1,6 @@
 import ActionSheet from 'packages/actionsheet';
 import { mount } from 'avoriaz';
+import { DOMChecker } from '../utils';
 
 describe('ActionSheet', () => {
   let wrapper;
@@ -26,7 +27,13 @@ describe('ActionSheet', () => {
       }
     });
 
-    expect(wrapper.instance().currentValue).to.be.true;
+    DOMChecker(wrapper, {
+      noStyle: {
+        '.van-actionsheet': {
+          display: 'none'
+        }
+      }
+    });
   });
 
   it('create title type actionsheet', () => {
@@ -98,8 +105,8 @@ describe('ActionSheet', () => {
       }
     });
 
-    const cancelButton = wrapper.find('.van-actionsheet__button')[0];
-    expect(wrapper.contains('.van-actionsheet__button')).to.be.true;
+    const cancelButton = wrapper.find('.van-actionsheet__cancel')[0];
+    expect(wrapper.contains('.van-actionsheet__cancel')).to.be.true;
     expect(cancelButton.text()).to.equal('cancel');
   });
 
@@ -111,12 +118,24 @@ describe('ActionSheet', () => {
     });
 
     const eventStub = sinon.stub(wrapper.vm, '$emit');
-    expect(wrapper.data().currentValue).to.be.false;
+    DOMChecker(wrapper, {
+      style: {
+        '.van-actionsheet': {
+          display: 'none'
+        }
+      }
+    });
 
     wrapper.vm.value = true;
     wrapper.update();
     wrapper.vm.$nextTick(() => {
-      expect(wrapper.data().currentValue).to.be.true;
+      DOMChecker(wrapper, {
+        noStyle: {
+          '.van-actionsheet': {
+            display: 'none'
+          }
+        }
+      });
       expect(eventStub.calledWith('input'));
       done();
     });
diff --git a/test/unit/utils.js b/test/unit/utils.js
new file mode 100644
index 000000000..b8d768787
--- /dev/null
+++ b/test/unit/utils.js
@@ -0,0 +1,46 @@
+/**
+ * 按照一定的规则进行匹配
+ */
+export function DOMChecker(wrapper, rules) {
+  const { text, count, src, style, noStyle, value } = rules;
+
+  if (text) {
+    Object.keys(text).forEach(key => {
+      expect(wrapper.find(key)[0].text().trim()).to.equal(text[key]);
+    });
+  }
+
+  if (count) {
+    Object.keys(count).forEach(key => {
+      expect(wrapper.find(key).length).to.equal(count[key]);
+    });
+  }
+
+  if (src) {
+    Object.keys(src).forEach(key => {
+      expect(wrapper.find(key)[0].element.src).to.equal(src[key]);
+    });
+  }
+
+  if (value) {
+    Object.keys(value).forEach(key => {
+      expect(wrapper.find(key)[0].element.value).to.equal(value[key]);
+    });
+  }
+
+  if (style) {
+    Object.keys(style).forEach(key => {
+      Object.keys(style[key]).forEach(prop => {
+        expect(wrapper.find(key)[0].hasStyle(prop, style[key][prop])).to.equal(true);
+      });
+    });
+  }
+
+  if (noStyle) {
+    Object.keys(noStyle).forEach(key => {
+      Object.keys(noStyle[key]).forEach(prop => {
+        expect(wrapper.find(key)[0].hasStyle(prop, noStyle[key][prop])).to.equal(false);
+      });
+    });
+  }
+}