Skip to content

Commit

Permalink
[MAJOR] use attribute hidden (#691)
Browse files Browse the repository at this point in the history
* use attribute hidden

* commit something to re-run tests
  • Loading branch information
tinovyatkin authored and jshjohnson committed Oct 24, 2019
1 parent 9bb0c62 commit 7887c05
Show file tree
Hide file tree
Showing 12 changed files with 60 additions and 74 deletions.
5 changes: 3 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
# Contributions
# Contributions
In lieu of a formal styleguide, take care to maintain the existing coding style ensuring there are no linting errors. Add unit tests for any new or changed functionality. Lint and test your code using the npm scripts below:

### NPM tasks
| Task | Usage |
| -------------------- | ------------------------------------------------------------ |
| `npm run start` | Fire up local server for development |
| `npm run test` | Run sequence of tests once |
| `npm run test:unit` | Run sequence of unit tests once |
| `npm run test:e2e` | Run sequence of integration tests once |
| `npm run test:watch` | Fire up test server and re-test on file change |
| `npm run js:build` | Compile Choices to an uglified JavaScript file |
| `npm run css:watch` | Watch SCSS files for changes. On a change, run build process |
Expand Down
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ will be returned. If you target one element, that instance will be returned.
openState: 'is-open',
disabledState: 'is-disabled',
highlightedState: 'is-highlighted',
hiddenState: 'is-hidden',
flippedState: 'is-flipped',
loadingState: 'is-loading',
noResults: 'has-no-results',
Expand Down Expand Up @@ -518,7 +517,6 @@ classNames: {
openState: 'is-open',
disabledState: 'is-disabled',
highlightedState: 'is-highlighted',
hiddenState: 'is-hidden',
flippedState: 'is-flipped',
selectedState: 'is-highlighted',
}
Expand Down
6 changes: 3 additions & 3 deletions cypress/integration/select-multiple.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ describe('Choices - select multiple', () => {

it('updates the value of the original input', () => {
cy.get('[data-test-hook=basic]')
.find('.choices__input.is-hidden')
.find('.choices__input[hidden]')
.should($select => {
expect($select.val()).to.contain(selectedChoiceText);
});
Expand Down Expand Up @@ -150,7 +150,7 @@ describe('Choices - select multiple', () => {

it('updates the value of the original input', () => {
cy.get('[data-test-hook=basic]')
.find('.choices__input.is-hidden')
.find('.choices__input[hidden]')
.should($select => {
const val = $select.val() || [];
expect(val).to.not.contain(removedChoiceText);
Expand Down Expand Up @@ -806,7 +806,7 @@ describe('Choices - select multiple', () => {

it('updates the value of the original input', () => {
cy.get('[data-test-hook=set-choice-by-value]')
.find('.choices__input.is-hidden')
.find('.choices__input[hidden]')
.should($select => {
const val = $select.val() || [];
expect(val).to.contain(dynamicallySelectedChoiceValue);
Expand Down
4 changes: 2 additions & 2 deletions cypress/integration/select-one.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ describe('Choices - select one', () => {

it('updates the value of the original input', () => {
cy.get('[data-test-hook=remove-button]')
.find('.choices__input.is-hidden')
.find('.choices__input[hidden]')
.should($select => {
const val = $select.val() || [];

Expand Down Expand Up @@ -818,7 +818,7 @@ describe('Choices - select one', () => {

it('updates the value of the original input', () => {
cy.get('[data-test-hook=set-choice-by-value]')
.find('.choices__input.is-hidden')
.find('.choices__input[hidden]')
.should($select => {
const val = $select.val() || [];
expect(val).to.contain(dynamicallySelectedChoiceValue);
Expand Down
4 changes: 2 additions & 2 deletions cypress/integration/text.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('Choices - text element', () => {
.type('{enter}');

cy.get('[data-test-hook=basic]')
.find('.choices__input.is-hidden')
.find('.choices__input[hidden]')
.should('have.value', textInput);
});

Expand Down Expand Up @@ -151,7 +151,7 @@ describe('Choices - text element', () => {
.click();

cy.get('[data-test-hook=remove-button]')
.find('.choices__input.is-hidden')
.find('.choices__input[hidden]')
.then($input => {
expect($input.val()).to.not.contain(textInput);
});
Expand Down
6 changes: 2 additions & 4 deletions src/scripts/components/wrapped-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export default class WrappedElement {
conceal() {
// Hide passed input
this.element.classList.add(this.classNames.input);
this.element.classList.add(this.classNames.hiddenState);
this.element.hidden = true;

// Remove element from tab index
this.element.tabIndex = '-1';
Expand All @@ -35,14 +35,13 @@ export default class WrappedElement {
this.element.setAttribute('data-choice-orig-style', origStyle);
}

this.element.setAttribute('aria-hidden', 'true');
this.element.setAttribute('data-choice', 'active');
}

reveal() {
// Reinstate passed element
this.element.classList.remove(this.classNames.input);
this.element.classList.remove(this.classNames.hiddenState);
this.element.hidden = false;
this.element.removeAttribute('tabindex');

// Recover original styles if any
Expand All @@ -54,7 +53,6 @@ export default class WrappedElement {
} else {
this.element.removeAttribute('style');
}
this.element.removeAttribute('aria-hidden');
this.element.removeAttribute('data-choice');

// Re-assign values - this is weird, I know
Expand Down
9 changes: 2 additions & 7 deletions src/scripts/components/wrapped-element.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,7 @@ describe('components/wrappedElement', () => {
expect(
instance.element.classList.contains(instance.classNames.input),
).to.equal(true);
expect(
instance.element.classList.contains(instance.classNames.hiddenState),
).to.equal(true);
expect(instance.element.getAttribute('aria-hidden')).to.equal('true');
expect(instance.element.hidden).to.be.true;
expect(instance.element.getAttribute('data-choice')).to.equal('active');
expect(instance.element.getAttribute('data-choice-orig-style')).to.equal(
originalStyling,
Expand All @@ -78,9 +75,7 @@ describe('components/wrappedElement', () => {
expect(
instance.element.classList.contains(instance.classNames.input),
).to.equal(false);
expect(
instance.element.classList.contains(instance.classNames.hiddenState),
).to.equal(false);
expect(instance.element.hidden).to.be.false;
expect(instance.element.getAttribute('style')).to.equal(originalStyling);
expect(instance.element.getAttribute('aria-hidden')).to.equal(null);
expect(instance.element.getAttribute('data-choice')).to.equal(null);
Expand Down
1 change: 0 additions & 1 deletion src/scripts/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ export const DEFAULT_CLASSNAMES = {
openState: 'is-open',
disabledState: 'is-disabled',
highlightedState: 'is-highlighted',
hiddenState: 'is-hidden',
flippedState: 'is-flipped',
loadingState: 'is-loading',
noResults: 'has-no-results',
Expand Down
1 change: 0 additions & 1 deletion src/scripts/constants.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ describe('constants', () => {
'openState',
'disabledState',
'highlightedState',
'hiddenState',
'flippedState',
'loadingState',
'noResults',
Expand Down
36 changes: 16 additions & 20 deletions src/styles/base.scss
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,23 @@
= Generic styling =
=============================================*/

$global-guttering : 24px;
$global-font-size-h1 : 32px;
$global-font-size-h2 : 24px;
$global-font-size-h3 : 20px;
$global-font-size-h4 : 18px;
$global-font-size-h5 : 16px;
$global-font-size-h6 : 14px;
$global-guttering: 24px;
$global-font-size-h1: 32px;
$global-font-size-h2: 24px;
$global-font-size-h3: 20px;
$global-font-size-h4: 18px;
$global-font-size-h5: 16px;
$global-font-size-h6: 14px;

* {
-webkit-font-smoothing: antialiased;
-moz-osx-font-smoothing: grayscale
-moz-osx-font-smoothing: grayscale;
}

*,
*:before,
*:after {
box-sizing: border-box
box-sizing: border-box;
}

html,
Expand All @@ -30,10 +30,10 @@ body {
}

body {
font-family: "Helvetica Neue", Helvetica, Arial, "Lucida Grande", sans-serif;
font-family: 'Helvetica Neue', Helvetica, Arial, 'Lucida Grande', sans-serif;
font-size: 16px;
line-height: 1.4;
color: #FFFFFF;
color: #ffffff;
background-color: #333;
overflow-x: hidden;
}
Expand All @@ -52,7 +52,7 @@ p {

hr {
display: block;
margin: $global-guttering*1.25 0;
margin: $global-guttering * 1.25 0;
border: 0;
border-bottom: 1px solid #eaeaea;
height: 1px;
Expand All @@ -73,7 +73,7 @@ h6 {
a,
a:visited,
a:focus {
color: #FFFFFF;
color: #ffffff;
text-decoration: none;
font-weight: 600;
}
Expand Down Expand Up @@ -133,14 +133,14 @@ label + p {
display: block;
margin: auto;
max-width: 40em;
padding: $global-guttering*2;
padding: $global-guttering * 2;
@media (max-width: 620px) {
padding: 0;
}
}

.section {
background-color: #FFFFFF;
background-color: #ffffff;
padding: $global-guttering;
color: #333;
a,
Expand Down Expand Up @@ -184,12 +184,8 @@ label + p {
text-align: center;
}

.is-hidden {
display: none;
}

[data-test-hook] {
margin-bottom: $global-guttering;
}

/*===== End of Section comment block ======*/
/*===== End of Section comment block ======*/
Loading

0 comments on commit 7887c05

Please sign in to comment.