Fix #727 (#731)

* Housekeeping * Resolve placeholder bug + hide from choice list * Restructure test folder * Update cypress test to assert one placeholder * Fix breaking e2e test * Remove ability to pass placeholder via config for select boxes * Add further e2e tests covering placeholders * Add unit tests for _generatePlaceholderValue * Display placeholder choice for select one * Add further e2e test to assert on placeholder ordering * Add labels to exclude from draft releases * Add failure case to e2e test workflow * Resolve broken e2e test * Update puppeteer snapshot baseline
Choices-js · Nov 2, 2019 · a0fe05f · a0fe05f
1 parent 939a73b
commit a0fe05f
Show file tree

Hide file tree

Showing 16 changed files with 398 additions and 70 deletions.
diff --git a/.github/actions-scripts/__snapshots__/puppeteer-darwin.png b/.github/actions-scripts/__snapshots__/puppeteer-darwin.png
diff --git a/.github/release-drafter.yml b/.github/release-drafter.yml
@@ -1,6 +1,9 @@
 name-template: 'Draft (next release)'
 tag-template: 'v$NEXT_PATCH_VERSION'
 sort-direction: descending
+exclude-labels:
+  - 'skip-changelog'
+  - 'release'
 categories:
   - title: '🚨 Breaking changes'
     labels:

diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml
@@ -29,7 +29,7 @@ jobs:
         env:
           HUSKY_SKIP_INSTALL: true
 
-      - name: run Cypress CI
+      - name: run Cypress (with recording)
         run: npx run-p --race start cypress:ci
         env:
           CI: true
@@ -41,3 +41,18 @@ jobs:
           COMMIT_INFO_BRANCH: ${{ github.head_ref }}
           COMMIT_INFO_AUTHOR: ${{ github.event.sender.login }}
           COMMIT_INFO_SHA: ${{ github.event.after }}
+
+      # if we have ran out of free Cypress recordings, run Cypress with recording switched off
+      - name: run Cypress (without recording)
+        if: failure()
+        run: npx run-p --race start cypress:run
+        env:
+          CI: true
+          TERM: xterm-256color
+          NODE_ENV: production # prevent watching
+          CYPRESS_RECORD_KEY: ${{ secrets.CYPRESS_RECORD_KEY }}
+          DEBUG: commit-info,cypress:server:record
+          # https://docs.cypress.io/guides/guides/continuous-integration.html#Environment-variables
+          COMMIT_INFO_BRANCH: ${{ github.head_ref }}
+          COMMIT_INFO_AUTHOR: ${{ github.event.sender.login }}
+          COMMIT_INFO_SHA: ${{ github.event.after }}
diff --git a/.github/workflows/release-management.yml → .github/workflows/release-drafter.yml b/.github/workflows/release-management.yml → .github/workflows/release-drafter.yml
@@ -1,4 +1,4 @@
-name: Release management
+name: Release drafter
 
 on:
   push:
@@ -9,6 +9,6 @@ jobs:
   update-draft-release:
     runs-on: ubuntu-latest
     steps:
-      - uses: toolmantim/release-drafter@v5.2.0
+      - uses: toolmantim/release-drafter@v5
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
diff --git a/README.md b/README.md
@@ -431,11 +431,11 @@ const example = new Choices(element, {
 
 **Type:** `Boolean` **Default:** `true`
 
-**Input types affected:** `text`, `select-multiple`
+**Input types affected:** `text`
 
 **Usage:** Whether the input should show a placeholder. Used in conjunction with `placeholderValue`. If `placeholder` is set to true and no value is passed to `placeholderValue`, the passed input's placeholder attribute will be used as the placeholder value.
 
-**Note:** For single select boxes, the recommended way of adding a placeholder is as follows:
+**Note:** For select boxes, the recommended way of adding a placeholder is as follows:
 
 ```html
 <select>

diff --git a/cypress/integration/select-multiple.spec.js b/cypress/integration/select-multiple.spec.js
@@ -1,6 +1,6 @@
 describe('Choices - select multiple', () => {
   beforeEach(() => {
-    cy.visit('/select-multiple.html');
+    cy.visit('/select-multiple');
   });
 
   describe('scenarios', () => {
@@ -486,20 +486,42 @@ describe('Choices - select multiple', () => {
       });
     });
 
-    describe('placeholder', () => {
-      /*
-        {
-          placeholder: true,
-          placeholderValue: 'I am a placeholder',
-        }
-      */
+    describe('placeholder via empty option value', () => {
+      describe('when no value has been inputted', () => {
+        it('displays a placeholder', () => {
+          cy.get('[data-test-hook=placeholder-via-option-value]')
+            .find('.choices__input--cloned')
+            .should('have.attr', 'placeholder', 'I am a placeholder');
+        });
+      });
+
+      describe('when a value has been inputted', () => {
+        it('does not display a placeholder', () => {
+          cy.get('[data-test-hook=placeholder-via-option-value]')
+            .find('.choices__input--cloned')
+            .type('test')
+            .should('not.have.value', 'I am a placeholder');
+        });
+      });
+    });
+
+    describe('placeholder via option attribute', () => {
       describe('when no value has been inputted', () => {
         it('displays a placeholder', () => {
-          cy.get('[data-test-hook=placeholder]')
+          cy.get('[data-test-hook=placeholder-via-option-attr]')
             .find('.choices__input--cloned')
             .should('have.attr', 'placeholder', 'I am a placeholder');
         });
       });
+
+      describe('when a value has been inputted', () => {
+        it('does not display a placeholder', () => {
+          cy.get('[data-test-hook=placeholder-via-option-attr]')
+            .find('.choices__input--cloned')
+            .type('test')
+            .should('not.have.value', 'I am a placeholder');
+        });
+      });
     });
 
     describe('remote data', () => {

diff --git a/cypress/integration/select-one.spec.js b/cypress/integration/select-one.spec.js
@@ -1,6 +1,6 @@
 describe('Choices - select one', () => {
   beforeEach(() => {
-    cy.visit('/select-one.html');
+    cy.visit('/select-one');
   });
 
   describe('scenarios', () => {
@@ -448,6 +448,102 @@ describe('Choices - select one', () => {
       });
     });
 
+    describe('placeholder via empty option value', () => {
+      describe('when no choice has been selected', () => {
+        it('displays a placeholder', () => {
+          cy.get('[data-test-hook=placeholder-via-option-value]')
+            .find('.choices__list--single')
+            .children()
+            .first()
+            .should('have.class', 'choices__placeholder')
+            .and($placeholder => {
+              expect($placeholder).to.contain('I am a placeholder');
+            });
+        });
+      });
+
+      describe('when a choice has been selected', () => {
+        it('does not display a placeholder', () => {
+          cy.get('[data-test-hook=placeholder-via-option-value]')
+            .find('.choices__input--cloned')
+            .focus();
+
+          cy.get('[data-test-hook=placeholder-via-option-value]')
+            .find('.choices__list--dropdown .choices__list')
+            .children()
+            .first()
+            .click();
+
+          cy.get('[data-test-hook=placeholder-via-option-value]')
+            .find('.choices__input--cloned')
+            .should('not.have.value', 'I am a placeholder');
+        });
+      });
+
+      describe('when choice list is open', () => {
+        it('displays the placeholder choice first', () => {
+          cy.get('[data-test-hook=placeholder-via-option-value]')
+            .find('.choices__input--cloned')
+            .focus();
+
+          cy.get('[data-test-hook=placeholder-via-option-value]')
+            .find('.choices__list--dropdown .choices__list')
+            .children()
+            .first()
+            .should('have.class', 'choices__placeholder')
+            .should('have.text', 'I am a placeholder');
+        });
+      });
+    });
+
+    describe('placeholder via option attribute', () => {
+      describe('when no choice has been selected', () => {
+        it('displays a placeholder', () => {
+          cy.get('[data-test-hook=placeholder-via-option-attr]')
+            .find('.choices__list--single')
+            .children()
+            .first()
+            .should('have.class', 'choices__placeholder')
+            .and($placeholder => {
+              expect($placeholder).to.contain('I am a placeholder');
+            });
+        });
+      });
+
+      describe('when a choice has been selected', () => {
+        it('does not display a placeholder', () => {
+          cy.get('[data-test-hook=placeholder-via-option-attr]')
+            .find('.choices__input--cloned')
+            .focus();
+
+          cy.get('[data-test-hook=placeholder-via-option-attr]')
+            .find('.choices__list--dropdown .choices__list')
+            .children()
+            .first()
+            .click();
+
+          cy.get('[data-test-hook=placeholder-via-option-attr]')
+            .find('.choices__input--cloned')
+            .should('not.have.value', 'I am a placeholder');
+        });
+      });
+
+      describe('when choice list is open', () => {
+        it('displays the placeholder choice first', () => {
+          cy.get('[data-test-hook=placeholder-via-option-attr]')
+            .find('.choices__input--cloned')
+            .focus();
+
+          cy.get('[data-test-hook=placeholder-via-option-attr]')
+            .find('.choices__list--dropdown .choices__list')
+            .children()
+            .first()
+            .should('have.class', 'choices__placeholder')
+            .should('have.text', 'I am a placeholder');
+        });
+      });
+    });
+
     describe('remote data', () => {
       beforeEach(() => {
         cy.reload(true);
@@ -458,6 +554,7 @@ describe('Choices - select one', () => {
           cy.get('[data-test-hook=remote-data]')
             .find('.choices__list--single')
             .children()
+            .should('have.length', 1)
             .first()
             .should('have.class', 'choices__placeholder')
             .and($placeholder => {
@@ -483,10 +580,14 @@ describe('Choices - select one', () => {
             cy.get('[data-test-hook=remote-data]')
               .find('.choices__list--dropdown .choices__list')
               .children()
-              .should('have.length', 50)
+              .should('have.length', 51) // 50 choices + 1 placeholder choice
               .each(($choice, index) => {
-                expect($choice.text().trim()).to.equal(`Label ${index + 1}`);
-                expect($choice.data('value')).to.equal(`Value ${index + 1}`);
+                if (index === 0) {
+                  expect($choice.text().trim()).to.equal('I am a placeholder');
+                } else {
+                  expect($choice.text().trim()).to.equal(`Label ${index}`);
+                  expect($choice.data('value')).to.equal(`Value ${index}`);
+                }
               });
           });
         });

diff --git a/cypress/integration/text.spec.js b/cypress/integration/text.spec.js
@@ -1,6 +1,6 @@
 describe('Choices - text element', () => {
   beforeEach(() => {
-    cy.visit('/text.html');
+    cy.visit('/text');
   });
 
   describe('scenarios', () => {

diff --git a/public/assets/styles/choices.css b/public/assets/styles/choices.css
@@ -218,7 +218,7 @@
 }
 
 .choices__list--dropdown {
-  display: none;
+  visibility: hidden;
   z-index: 1;
   position: absolute;
   width: 100%;
@@ -230,10 +230,11 @@
   border-bottom-right-radius: 2.5px;
   overflow: hidden;
   word-break: break-all;
+  will-change: visibility;
 }
 
 .choices__list--dropdown.is-active {
-  display: block;
+  visibility: visible;
 }
 
 .is-open .choices__list--dropdown {