Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Hide Accordion closed Sections instead of unmounting them #2149

Merged
merged 10 commits into from
Feb 11, 2020
+65 −22
Merged

Hide Accordion closed Sections instead of unmounting them #2149

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d84bfa7
Hides section contents instead of unmounting them
supernova-at Feb 7, 2020
47f4c33
Fixes up tests. Closes all price adjustement sections by default.
supernova-at Feb 7, 2020
a4f5f99
Runs prettier
supernova-at Feb 7, 2020
2c6024d
Merge remote-tracking branch 'origin/develop' into supernova/360_acco…
supernova-at Feb 10, 2020
91ef9f3
Updates priceAdjustments snapshot. All section children should be pre…
supernova-at Feb 10, 2020
ffc80a9
Merge branch 'develop' into supernova/360_accordion
supernova-at Feb 10, 2020
a782920
Merge branch 'develop' into supernova/360_accordion
dpatil-magento Feb 10, 2020
710ed9e
Merge branch 'develop' into supernova/360_accordion
dpatil-magento Feb 10, 2020
3ed75ef
Updates priceAdjustments unit test and snapshot
supernova-at Feb 10, 2020
637fbbd
Merge branch 'develop' into supernova/360_accordion
supernova-at Feb 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 35 additions & 11 deletions packages/venia-ui/lib/components/Accordion/__tests__/__snapshots__/accordion.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,24 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`it renders a closed Section correctly 1`] = `
<div>
<div>
<div
className="root"
>
<div
className="root"
>
<button
className="title_container"
onClick={[Function]}
>
<div>
<div
className="title"
>
The Section Title is always visible
</div>
<span>
<span
className="root"
>
<svg
fill="none"
height="24"
Expand All @@ -27,21 +36,34 @@ exports[`it renders a closed Section correctly 1`] = `
</svg>
</span>
</button>
<div />
<div
className="contents_container_closed"
>
This section is closed. Its className should match.
</div>
</div>
</div>
`;

exports[`it renders an open Section correctly 1`] = `
<div>
<div>
<div
className="root"
>
<div
className="root"
>
<button
className="title_container_open"
onClick={[Function]}
>
<div>
<div
className="title"
>
The Section Title is always visible
</div>
<span>
<span
className="root"
>
<svg
fill="none"
height="24"
Expand All @@ -59,8 +81,10 @@ exports[`it renders an open Section correctly 1`] = `
</svg>
</span>
</button>
<div>
This section is open. You should see this content.
<div
className="contents_container"
>
This section is open. Its className should match.
</div>
</div>
</div>
Expand Down
6 changes: 4 additions & 2 deletions packages/venia-ui/lib/components/Accordion/__tests__/accordion.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@ import { createTestInstance } from '@magento/peregrine';
import Accordion from '../accordion';
import Section from '../section';

jest.mock('../../../classify');

test('it renders a closed Section correctly', () => {
// Act.
const instance = createTestInstance(
<Accordion>
<Section title="The Section Title is always visible">
This section is closed. You should not see this content.
This section is closed. Its className should match.
</Section>
</Accordion>
);
Expand All @@ -23,7 +25,7 @@ test('it renders an open Section correctly', () => {
const instance = createTestInstance(
<Accordion>
<Section title="The Section Title is always visible" isOpen={true}>
This section is open. You should see this content.
This section is open. Its className should match.
</Section>
</Accordion>
);
Expand Down
5 changes: 5 additions & 0 deletions packages/venia-ui/lib/components/Accordion/section.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@
.contents_container {
padding: 0 1.5rem 1.5rem;
}

.contents_container_closed {
display: none;
}

.contents_container:empty {
display: none;
}
Expand Down
6 changes: 4 additions & 2 deletions packages/venia-ui/lib/components/Accordion/section.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ const Section = props => {
);

const isOpen = openSectionIds.has(id);
const contents = isOpen ? children : null;
const titleIconSrc = isOpen ? ArrowUp : ArrowDown;
const titleIcon = <Icon src={titleIconSrc} />;

const classes = mergeClasses(defaultClasses, props.classes);
const contentsContainerClass = isOpen
? classes.contents_container
: classes.contents_container_closed;
const titleContainerClass = isOpen
? classes.title_container_open
: classes.title_container;
Expand All @@ -36,7 +38,7 @@ const Section = props => {
<div className={classes.title}>{title}</div>
{titleIcon}
</button>
<div className={classes.contents_container}>{contents}</div>
<div className={contentsContainerClass}>{children}</div>
</div>
);
};
Expand Down
16 changes: 13 additions & 3 deletions ...omponents/CartPage/PriceAdjustments/__tests__/__snapshots__/priceAdjustments.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,13 @@ exports[`it renders Venia price adjustments 1`] = `
</svg>
</span>
</button>
<div />
<div>
<a
href="https://jira.corp.magento.com/browse/PWA-239"
>
Shipping Methods to be completed by PWA-239.
</a>
</div>
</div>
<div>
<button
Expand All @@ -55,7 +61,9 @@ exports[`it renders Venia price adjustments 1`] = `
</svg>
</span>
</button>
<div />
<div>
<CouponCode />
</div>
</div>
<GiftCardSection />
<div>
Expand Down Expand Up @@ -83,7 +91,9 @@ exports[`it renders Venia price adjustments 1`] = `
</svg>
</span>
</button>
<div />
<div>
<GiftOptions />
</div>
</div>
</div>
</div>
Expand Down
2 changes: 2 additions & 0 deletions ...ages/venia-ui/lib/components/CartPage/PriceAdjustments/__tests__/priceAdjustments.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import { createTestInstance } from '@magento/peregrine';

import PriceAdjustments from '../priceAdjustments';

jest.mock('../CouponCode', () => 'CouponCode');
jest.mock('../giftCardSection', () => 'GiftCardSection');
jest.mock('../GiftOptions', () => 'GiftOptions');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that the children get rendered regardless of whether their section is open or not, we have to mock them all.


test('it renders Venia price adjustments', () => {
// Act.
Expand Down
6 changes: 2 additions & 4 deletions packages/venia-ui/lib/components/CartPage/PriceAdjustments/priceAdjustments.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
import React from 'react';

import { Accordion, Section } from '../../Accordion';

import CouponCode from './CouponCode';
import GiftCardSection from './giftCardSection';
import GiftOptions from './GiftOptions';

import { mergeClasses } from '../../../classify';
import defaultClasses from './priceAdjustments.css';
import CouponCode from './CouponCode';
import GiftCardSection from './giftCardSection';

const PriceAdjustments = props => {
const classes = mergeClasses(defaultClasses, props.classes);

// TODO: Minimizing accordion views actually unmounts the components. If a component does things, like make a query, on mount, it may make unnecessary queries. Can we just hide the content?
return (
<div className={classes.root}>
<Accordion canOpenMultiple={true}>
Expand Down