Commit 6ef66155 authored by Lyza Danger Gardner's avatar Lyza Danger Gardner Committed by Lyza Gardner

Use `IconButton`, `LabeledButton` in `Thread`

Replace uses of `Button` in the Thread component and adjust
styling for stronger alignment in both desktop and touch interfaces.
parent 0edbc1ba
...@@ -5,8 +5,9 @@ import { useStoreProxy } from '../store/use-store'; ...@@ -5,8 +5,9 @@ import { useStoreProxy } from '../store/use-store';
import { withServices } from '../service-context'; import { withServices } from '../service-context';
import { countHidden, countVisible } from '../helpers/thread'; import { countHidden, countVisible } from '../helpers/thread';
import { IconButton, LabeledButton } from '../../shared/components/buttons';
import Annotation from './Annotation'; import Annotation from './Annotation';
import Button from './Button';
import ModerationBanner from './ModerationBanner'; import ModerationBanner from './ModerationBanner';
/** @typedef {import('../helpers/build-thread').Thread} Thread */ /** @typedef {import('../helpers/build-thread').Thread} Thread */
...@@ -90,12 +91,17 @@ function Thread({ thread, threadsService }) { ...@@ -90,12 +91,17 @@ function Thread({ thread, threadsService }) {
> >
{showThreadToggle && ( {showThreadToggle && (
<div className="Thread__collapse"> <div className="Thread__collapse">
<Button <div className="Thread__collapse-button-container">
className="Thread__collapse-button" <IconButton
icon={toggleIcon} className="NonResponsiveIconButton"
title={toggleTitle} expanded={!thread.collapsed}
onClick={onToggleReplies} icon={toggleIcon}
/> title={toggleTitle}
onClick={onToggleReplies}
size="medium"
variant="light"
/>
</div>
</div> </div>
)} )}
...@@ -103,11 +109,11 @@ function Thread({ thread, threadsService }) { ...@@ -103,11 +109,11 @@ function Thread({ thread, threadsService }) {
{annotationContent} {annotationContent}
{showHiddenToggle && ( {showHiddenToggle && (
<Button <div className="Thread__hidden-toggle-button-container">
buttonText={`Show ${countHidden(thread)} more in conversation`} <LabeledButton onClick={() => threadsService.forceVisible(thread)}>
className="Thread__hidden-toggle-button" Show {countHidden(thread)} more in conversation
onClick={() => threadsService.forceVisible(thread)} </LabeledButton>
/> </div>
)} )}
{showChildren && ( {showChildren && (
......
...@@ -111,7 +111,7 @@ describe('Thread', () => { ...@@ -111,7 +111,7 @@ describe('Thread', () => {
// Retrieve the (caret) button for showing and hiding replies // Retrieve the (caret) button for showing and hiding replies
const getToggleButton = wrapper => { const getToggleButton = wrapper => {
return wrapper.find('Button').filter('.Thread__collapse-button'); return wrapper.find('IconButton');
}; };
beforeEach(() => { beforeEach(() => {
...@@ -178,7 +178,9 @@ describe('Thread', () => { ...@@ -178,7 +178,9 @@ describe('Thread', () => {
collapsedThread.parent = '1'; collapsedThread.parent = '1';
const wrapper = createComponent({ thread: collapsedThread }); const wrapper = createComponent({ thread: collapsedThread });
assert.isTrue(wrapper.find('.Thread__collapse-button').exists()); assert.isTrue(
wrapper.find('IconButton[title="Expand replies"]').exists()
);
}); });
it('does not render child threads', () => { it('does not render child threads', () => {
...@@ -231,11 +233,7 @@ describe('Thread', () => { ...@@ -231,11 +233,7 @@ describe('Thread', () => {
const wrapper = createComponent({ thread }); const wrapper = createComponent({ thread });
act(() => { act(() => {
wrapper wrapper.find('LabeledButton').props().onClick();
.find('Button')
.filter({ buttonText: 'Show 1 more in conversation' })
.props()
.onClick();
}); });
assert.calledOnce(fakeThreadsService.forceVisible); assert.calledOnce(fakeThreadsService.forceVisible);
......
...@@ -34,3 +34,12 @@ ...@@ -34,3 +34,12 @@
padding: var.$layout-space--small var.$layout-space; padding: var.$layout-space--small var.$layout-space;
} }
} }
// An IconButton, but override minimum height/width on touch screen devices
.NonResponsiveIconButton {
@include buttons.IconButton(
(
'responsive': false,
)
);
}
@use "../../mixins/buttons";
@use "../../mixins/layout"; @use "../../mixins/layout";
@use "../../variables" as var; @use "../../variables" as var;
...@@ -13,19 +12,17 @@ ...@@ -13,19 +12,17 @@
margin-left: var.$layout-space; margin-left: var.$layout-space;
} }
&__children {
margin-left: -1 * var.$layout-space;
}
&__unavailable-message {
margin: 0 var.$layout-space;
}
// Left "channel" of thread // Left "channel" of thread
&__collapse { &__collapse {
margin-right: 1em; position: relative;
// Ensure that even when this thread is collapsed, it is allotted a minimal
// bit of vertical real estate so that the expand/collapse toggle (which is
// positioned absolute) has "room". This applies when collapsing a thread
// with a deleted annotation in it, as otherwise there is no content when
// collapsed.
min-height: 2em;
border-right: 1px dashed var.$color-border; border-right: 1px dashed var.$color-border;
// The entire channel is NOT clickable so don't make it look like it is // The entire channel is not clickable so don't make it look like it is
// (overrides `pointer` cursor applied to entire card) // (overrides `pointer` cursor applied to entire card)
cursor: auto; cursor: auto;
...@@ -39,30 +36,23 @@ ...@@ -39,30 +36,23 @@
} }
} }
&__collapse-button { // Container holding the thread's collapse/expand "chevron" icon button
@include buttons.button; &__collapse-button-container {
color: var.$grey-mid; position: absolute;
margin-right: -1.25em; // Position this element (and its button) such that it is horizontally
padding: 0.25em 0.75em 0.75em 0.75em; // centered on the parent element's right border and aligned vertically
// Need a non-transparent background so that the dashed border line // with the content on the right
// does not show through the button top: -0.25em;
left: -1em;
// Give a non-transparent background to this element so that the dashed
// border (vertical) on the parent element doesn't show through the
// child button
background-color: var.$white; background-color: var.$white;
svg {
// This is not an icon and as such does not use a mixin
width: 12px;
height: 12px;
color: var.$grey-4;
}
&:hover &__icon {
color: var.$grey-6;
}
} }
&__hidden-toggle-button {
// This makes the vertical alignment with thread-collapse chevrons &__hidden-toggle-button-container {
// more precise // This makes the vertical alignment of the "Show x more in conversation"
@include buttons.button--labeled; // button more precise with thread-collapse chevrons
margin-top: -0.25em; margin-top: -0.25em;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment