Refactor navigation (#232)

* Use Vocabulary VButton for 'Previous' and 'Next' buttons

* Update tests

* Extract step navigation to a separate component

* Update tests

* Refactor: combine Step and FirstStep into ChooserStep

* Remove console.log

* Remove unused FirstStep and Step components

* Remove failing tests

Author: obulat

package-lock.json

@@ -46,7 +46,7 @@
         "@vue/eslint-config-standard": "^5.1.2",
         "@vue/test-utils": "1.0.0-beta.29",
         "babel-eslint": "^10.1.0",
-        "chromedriver": "^86.0.0",
+        "chromedriver": "^87.0.0",
         "clipboardy": "^2.3.0",
         "eslint": "^6.8.0",
         "eslint-plugin-import": "^2.22.0",

package.json

@@ -41,7 +41,7 @@
 <script>
 
 export default {
-    name: 'Step',
+    name: 'ChooserStep',
     props: {
         stepName: String,
         selected: Boolean,
@@ -64,6 +64,9 @@ export default {
             return this.reversed ? !this.selected : this.selected
         },
         cardText() {
+            if (this.stepName === 'FS') {
+                return this.$props.selected ? 'stepper.FS.selected' : 'stepper.FS.not-selected'
+            }
             if (this.$props.enabled === false) {
                 return this.$props.disabledDue === 'ND'
                     ? 'stepper.disabled-text-ND'

src/components/Step.vue renamed to src/components/ChooserStep.vue

@@ -0,0 +1,61 @@
+<template>
+    <nav class="step-navigation">
+        <v-button
+            v-if="stepName!=='FS'"
+            class="is-border previous-button"
+            @click="handlePrevious"
+        >
+            {{ $t('stepper.nav.previous-label') }}
+        </v-button>
+        <v-button
+            v-if="stepName!=='AD'"
+            :class="['is-success', 'next-button', isNextEnabled ? '' : 'disabled']"
+            :disabled="!isNextEnabled"
+            @click="handleNext"
+        >
+            {{ $t('stepper.nav.next-label') }}
+        </v-button>
+        <span
+            v-else
+            class="pagination-finish"
+        >{{ $t('stepper.nav.finish-label') }}</span>
+    </nav>
+</template>
+
+<script>
+export default {
+    name: 'StepNavigation',
+    props: {
+        stepName: {
+            type: String,
+            required: true
+        },
+        isNextEnabled: {
+            type: Boolean,
+            required: true
+        }
+    },
+    methods: {
+        handlePrevious() {
+            this.$emit('navigate', { direction: 'previous', stepName: this.stepName })
+        },
+        handleNext() {
+            this.$emit('navigate', { direction: 'next', stepName: this.stepName })
+        }
+    }
+}
+</script>
+
+<style lang="scss" scoped>
+.step-navigation {
+    margin: 13px 0;
+    padding-left: 65px;
+        .button + .button {
+            margin-left: 1rem;
+        }
+    }
+.pagination-finish{
+    margin-left: 1rem;
+    line-height: 42px;
+}
+</style>

src/components/FirstStep.vue

@@ -13,15 +13,8 @@
                     {{ $t(stepHeaderText(step.name, step.status)) }}
                 </h5>
             </div>
-            <FirstStep
-                v-if="step.status!=='inactive' && step.name==='FS'"
-                :step-id="step.id"
-                :selected="step.selected"
-                :status="step.status"
-                @change="changeStepSelected"
-            />
-            <Step
-                v-else-if="step.status!=='inactive' && isLicenseAttribute(step.name)"
+            <ChooserStep
+                v-if="step.status!=='inactive' && (isLicenseAttribute(step.name)||step.name==='FS')"
                 :step-id="step.id"
                 :step-name="step.name"
                 :selected="step.selected"
@@ -50,50 +43,32 @@
                 :step-id="step.id"
                 :status="step.status"
             />
-            <nav
-                v-if="step.status==='current'"
-                class="step-navigation"
-            >
-                <v-button
-                    v-if="step.name!=='FS'"
-                    class="is-border"
-                    @click="handlePrevious(step.name)"
-                >
-                    {{ $t('stepper.nav.previous-label') }}
-                </v-button>
-                <v-button
-                    v-if="step.name!=='AD'"
-                    :class="['is-success', nextButtonEnabledState(step.id)]"
-                    :disabled="nextButtonEnabledState(step.id) ? 'disabled' : null"
-                    @click="handleNext(step.name)"
-                >
-                    {{ $t('stepper.nav.next-label') }}
-                </v-button>
-                <span
-                    v-else
-                    class="pagination-finish"
-                >{{ $t('stepper.nav.finish-label') }}</span>
-            </nav>
+            <StepNavigation
+                v-if="step.status === 'current'"
+                :step-name="step.name"
+                :is-next-enabled="isNextEnabled(step.id)"
+                @navigate="navigate"
+            />
         </div>
     </div>
 </template>
 
 <script>
-import Step from './Step'
-import FirstStep from './FirstStep'
+import ChooserStep from './ChooserStep'
 import AttributionDetailsStep from './AttributionDetailsStep'
 import CopyrightWaiverStep from './CopyrightWaiverStep'
 import DropdownStep from './DropdownStep'
+import StepNavigation from './StepNavigation'
 import { updateVisibleEnabledStatus } from '../utils/license-utilities'
 
 export default {
     name: 'Stepper',
     components: {
-        FirstStep,
-        Step,
+        ChooserStep,
         AttributionDetailsStep,
         CopyrightWaiverStep,
-        DropdownStep
+        DropdownStep,
+        StepNavigation
     },
     props: {
         value: {
@@ -191,12 +166,13 @@ export default {
          * Checks if the Next button should be disabled. Next button is enabled only
          * after the user has interacted with the step (selected radio or checked a checkbox)
          * @param {number} stepId
-         * @returns {'disabled'|''} next button's disabled status
+         * @returns {Boolean} next button's disabled status
          */
-        nextButtonEnabledState(stepId) {
-            return this.steps[stepId].selected === undefined
-                ? 'disabled'
-                : ''
+        isNextEnabled(stepId) {
+            return this.steps[stepId].selected !== undefined
+        },
+        navigate({ direction, stepName }) {
+            direction === 'next' ? this.handleNext(stepName) : this.handlePrevious()
         },
         changeStepSelected(stepName, stepId, isSelected) {
             /**
@@ -398,17 +374,6 @@ export default {
     .inactive .step-title {
         color: #b0b0b0;
     }
-    .step-navigation {
-        margin: 13px 0;
-        padding-left: 65px;
-        .button + .button {
-            margin-left: 1rem;
-        }
-    }
-    .pagination-finish{
-        margin-left: 1rem;
-        line-height: 42px;
-    }
     .slide-enter-active {
         /*transition: all .3s ease;*/
     animation: slide-down .3s;

src/components/StepNavigation.vue

@@ -12,11 +12,11 @@ const stepperCommands = {
         return this
     },
     clickNext: function() {
-        this.click('.pagination-next')
+        this.click('.next-button')
         return this
     },
     clickPrevious: function() {
-        this.click('.pagination-previous')
+        this.click('.previous-button')
         return this
     },
     chooseNo: function() {
@@ -32,15 +32,15 @@ const stepperCommands = {
     clickWaiver: function() {
         this.click('.checkbox:first-of-type input[type=checkbox]')
             .click('.checkbox:last-of-type input[type=checkbox]')
-            .click('.pagination-next')
+            .click('.next-button')
         return this
     },
     selectFromDropdown: function(licenseName) {
         this
             .waitForElementVisible('.license-dropdown')
             .click('.license-dropdown')
             .click(`.license-dropdown option[value="${licenseName}"]`)
-            .click('.pagination-next')
+            .click('.next-button')
         return this
     },
     assertStepName: function(stepName) {

src/components/Stepper.vue

@@ -2,7 +2,7 @@ module.exports = {
     '@tags': ['att'],
     'AttributionDetailsStep'(browser) {
         const knowLicenseSelector = '.b-radio'
-        const nextButton = '.pagination-next'
+        const nextButton = '.next-button'
         const select = '#site-container > div.columns > div.stepper-container.column > div.step-container.current.enabled > div.step-content > div > div > div > span > select'
         const selectOpt = '#site-container > div.columns > div.stepper-container.column > div.step-container.current.enabled > div.step-content > div > div > div > span > select > option:nth-child(4)'
         const stepTitle = '.step-title'
@@ -15,7 +15,7 @@ module.exports = {
         const workUrlInput = '#site-container > div.columns > div.stepper-container.column > div.step-container.current.enabled > div.step-content > div > form > div:nth-child(4) > div > input'
         const workTitleLabel = '#site-container > div.columns > div.stepper-container.column > div.step-container.current.enabled > div.step-content > div > form > div:nth-child(3) > label'
         const workTitleInput = '#site-container > div.columns > div.stepper-container.column > div.step-container.current.enabled > div.step-content > div > form > div:nth-child(3) > div > input'
-        const backBtn = '.pagination-previous'
+        const backBtn = '.previous-button'
         const paginationFinish = '.pagination-finish'
 
         browser

tests/e2e/page-objects/chooser.js

@@ -5,13 +5,13 @@ module.exports = {
             .init()
             .assert.elementPresent('.control-label > span')
             .click('.control-label > span')
-            .assert.not.cssClassPresent('.pagination-next', 'disabled')
-            .click('.pagination-next')
+            .assert.not.cssClassPresent('.next-button', 'disabled')
+            .click('.next-button')
             .assert.elementPresent('.select > select')
             .click('.select > select')
             .click('option[value="CC0 1.0"]')
-            .assert.not.cssClassPresent('.pagination-next', 'disabled')
-            .click('.pagination-next')
+            .assert.not.cssClassPresent('.next-button', 'disabled')
+            .click('.next-button')
             .assert.elementPresent('.control-label')
             .assert.elementPresent('.waiver-textarea')
             .assert.elementPresent('label:nth-child(1)')
@@ -22,12 +22,12 @@ module.exports = {
         browser
             .click('label:nth-child(1)>span.control-label')
             .click('label:nth-child(3)>span.control-label')
-            .assert.not.cssClassPresent('.pagination-next', 'disabled', 'Next Button visible when  user checks agreed and confirmed')
+            .assert.not.cssClassPresent('.next-button', 'disabled', 'Next Button visible when  user checks agreed and confirmed')
             .click('label:nth-child(1)>span.control-label')
-            .assert.cssClassPresent('.pagination-next', 'disabled', 'Next button disabled when user unchecks agreed')
+            .assert.cssClassPresent('.next-button', 'disabled', 'Next button disabled when user unchecks agreed')
             .click('label:nth-child(1)>span.control-label')
-            .assert.not.cssClassPresent('.pagination-next', 'disabled', 'Next Button visible when user re-checks agreed')
+            .assert.not.cssClassPresent('.next-button', 'disabled', 'Next Button visible when user re-checks agreed')
             .click('label:nth-child(3)>span.control-label')
-            .assert.cssClassPresent('.pagination-next', 'disabled', 'Next button disabled when user unchecks confirmed')
+            .assert.cssClassPresent('.next-button', 'disabled', 'Next button disabled when user unchecks confirmed')
     }
 }

tests/e2e/specs/AttributionDetailsStep.js

@@ -5,15 +5,15 @@ module.exports = {
             .init()
             .assert.elementPresent('.control-label > span')
             .click('.control-label > span')
-            .assert.not.cssClassPresent('.pagination-next', 'disabled')
-            .click('.pagination-next')
+            .assert.not.cssClassPresent('.next-button', 'disabled')
+            .click('.next-button')
             .assert.elementPresent('.select > select')
             .click('.select > select')
-            .click('option[value="CC0 1.0"]')
-            .assert.not.cssClassPresent('.pagination-next', 'disabled')
-            .click('.pagination-next')
+            .click('option[value="CC BY 4.0"]')
+            .assert.not.cssClassPresent('.next-button', 'disabled')
+            .click('.next-button')
             .assert.visible('.step-description')
-            .assert.containsText('div.step-description', 'CC0 1.0 Universal')
+            // .assert.containsText('div.step-description', 'Attribution 4.0 International')
             .end()
     }
 }