Feature/loader #67

kjugi · 2018-06-08T15:50:14Z

Add loader to button while clicking on it after filling up form with data. Must validate first and then showing loader and blocking button to prevent multiple clicks and sending multiple requests.

Modify button props.
Add styles to button
Create loader components with styles

…ify loader icon

szafran89 · 2018-06-14T07:46:33Z

view/frontend/web/js/components/BaseButton.vue

+    class="button"
+    :type="buttonType"
+    :title="buttonTitle"
+    :disabled="isBlocked"


maybe we can set disabled button if prop withLoader is true?

prop withLoader is not a loader state. It's like a variant for the button. You can set button without or with loader. So I can't disable the button when withLoader is true b/c it will be disabled it on start

szafran89 · 2018-06-14T07:46:53Z

view/frontend/web/js/components/BaseButton.vue

+  components: {
+    BaseLoader
+  },
+  props: ['buttonType', 'text', 'buttonTitle', 'withLoader'],


Why did you change props? Prev version was ok. We should add require property.

Maybe it would be better to cahnge prop withLoader to loader

I think prop loader means it will enable loader or start the loader. In this situation, withLoader means it will show the button with loader option. You can also show normal button without it

And I changed props b/c prev we just have type: String without require property

So better is to add require true or false

szafran89 · 2018-06-14T07:47:30Z

view/frontend/web/js/components/BaseButton.vue

+
+      <template v-if="withLoader">
+        <BaseLoader/>
+      </template>


Can you move v-if="withLoader" to BaseLoader

b/c it's variant for button. Not always you want to show button with loader. I.e when have 2 buttons and one is "Place order" and the second is "Back"

But it does not metter you can move condition to BaseLoader, like below

<BaseLoader v-if="withLoader"/>

in this case we dont need temlate tag

szafran89 · 2018-06-14T07:48:10Z

view/frontend/web/js/components/BaseButton.vue

+  }
+
+  &:disabled {
+    opacity: .5;


I know it works but please change to opacity: 0.5;

szafran89 · 2018-06-14T07:49:10Z

view/frontend/web/js/components/BaseLoader.vue

+          begin="0.3"
+        />
+      </circle>
+    </svg>


please minify svg

added svg-loader and add icon to assets + minify it and change styles to non scoped to get it in loader

szafran89 · 2018-06-14T07:50:27Z

view/frontend/web/js/store/index.js

@@ -51,7 +51,8 @@ const store = new Vuex.Store({
      shippingCarrierCode: '',
      shippingMethodCode: ''
    },
-    totals: null
+    totals: {},


why you changed to empty object?

And want to see it it working or no. For now we are not setting to totals anything. Is it ok?

yes, for now we don't use this state

szafran89 · 2018-06-14T08:34:21Z

view/frontend/web/js/components/steps/TheShippingStep.vue

+      this.$store.commit('setLoading', true)
+      this.regions = this.$store.getters.regionsByCountryId(this.address.country_id)
+      this.$store.dispatch('updateShippingMethods', this.address.country_id)
+    },


Do we need this method. Where do you execute it?

It looks like some dead code - deleted

szafran89 · 2018-06-18T22:54:27Z

view/frontend/web/js/components/steps/TheShippingStep.vue

@@ -61,7 +61,7 @@
          In this country we don't handle any shipping methods.
        </p>
      </template>
-      <BaseButton class="button" button-type="submit" text="Next Step"/>
+      <BaseButton class="button" button-type="submit" text="Next Step" with-loader="true"/>


you can change with-loader="true" to with-loader

szafran89 · 2018-06-18T22:58:21Z

view/frontend/web/js/components/steps/ThePaymentStep.vue

@@ -50,6 +50,7 @@
      class="button"
      button-type="button"
      text="Place order"
+      with-loader="true"


you can change with-loader="true" to with-loader

szafran89 · 2018-06-18T23:03:19Z

view/frontend/web/js/components/steps/ThePaymentStep.vue

@@ -105,13 +106,19 @@ export default {
        this.$validator.validateAll().then((result) => {
          if (result) {
            EventBus.$emit('save-address', 'billingAddress')
+            this.$store.commit('setLoading', true)
+            this.$store.commit('setAddress', {


Why did you add above this commit?

Conflicts: view/frontend/web/js/components/BaseButton.vue view/frontend/web/js/components/steps/TheShippingStep.vue yarn.lock

kjugi · 2018-07-10T10:37:07Z

If we want to add loader on start than must deal with v-cloak and add some styles to loader

szafran89 · 2018-07-10T17:13:08Z

view/frontend/web/js/assets/loader.svg

@@ -0,0 +1 @@
+<svg xmlns="http://www.w3.org/2000/svg"><circle cx="4" cy="4" r="4"><animate attributeName="opacity" dur="1s" values="0;1;0" repeatCount="indefinite" begin=".1"/></circle><circle cx="18" cy="4" r="4"><animate attributeName="opacity" dur="1s" values="0;1;0" repeatCount="indefinite" begin=".2"/></circle><circle cx="32" cy="4" r="4"><animate attributeName="opacity" dur="1s" values="0;1;0" repeatCount="indefinite" begin=".3"/></circle></svg>


missing new line :D

szafran89 · 2018-07-10T17:15:30Z

view/frontend/web/js/components/BaseButton.vue

+
+      <template v-if="withLoader">
+        <BaseLoader/>
+      </template>


But it does not metter you can move condition to BaseLoader, like below

<BaseLoader v-if="withLoader"/>

in this case we dont need temlate tag

szafran89 · 2018-07-10T17:18:08Z

@kjugi just 2 comments to fix ;)

kjugi · 2018-07-11T13:29:28Z

Closes #26

Filip Kudła added 5 commits April 27, 2018 19:55

Add base loader with mutation

dc848d8

Add baseLoader commits to store

f12c09e

Merge branch 'develop' into feature/loader

9a11e95

Delete loader on whole site and add indentation in config

a024096

Add loader to button and add attribute to it + styles to button + mod…

179ba53

…ify loader icon

szafran89 reviewed Jun 14, 2018

View reviewed changes

Filip Kudła added 5 commits June 15, 2018 17:21

Delete dead code - old method

3cf165e

Revert totals to null in default

8af448b

Add vue-svg-loader

b1df1f2

Add fresh yarn.lock

16b0ae1

Add svg icon and adjust Loader component to it

400aa0e

kjugi self-assigned this Jun 15, 2018

szafran89 reviewed Jun 18, 2018

View reviewed changes

Filip Kudła added 4 commits July 9, 2018 13:47

Merge remote-tracking branch 'refs/remotes/composer/develop'

c1a9401

Conflicts: view/frontend/web/js/components/BaseButton.vue view/frontend/web/js/components/steps/TheShippingStep.vue yarn.lock

remove true parameter b/c it's not required

0d7bf0a

Remove old setAddress commit to store

71ba809

Repair linter errors

35334b0

szafran89 reviewed Jul 10, 2018

View reviewed changes

szafran89 added 5 commits July 11, 2018 14:32

Updated button component, removed duplicated 'button' class

98707d1

Added missing new line at the end of file

c1b5b13

Updated vue-test-utils version

fd63583

Moved svg folder, updated yarn.lock

eac3210

Updated test snapshot

ed59198

kjugi merged commit 8a71f51 into develop Jul 11, 2018

kjugi deleted the feature/loader branch July 11, 2018 13:30

		@@ -0,0 +1 @@
		<svg xmlns="http://www.w3.org/2000/svg"><circle cx="4" cy="4" r="4"><animate attributeName="opacity" dur="1s" values="0;1;0" repeatCount="indefinite" begin=".1"/></circle><circle cx="18" cy="4" r="4"><animate attributeName="opacity" dur="1s" values="0;1;0" repeatCount="indefinite" begin=".2"/></circle><circle cx="32" cy="4" r="4"><animate attributeName="opacity" dur="1s" values="0;1;0" repeatCount="indefinite" begin=".3"/></circle></svg>

Feature/loader #67

Feature/loader #67

Conversation

kjugi commented Jun 8, 2018 • edited Loading

Choose a reason for hiding this comment

kjugi Jun 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kjugi commented Jul 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

szafran89 commented Jul 10, 2018

kjugi commented Jul 11, 2018

kjugi commented Jun 8, 2018 •

edited

Loading

kjugi Jun 15, 2018 •

edited

Loading