Skip to content

Commit c0d44db

Browse files
committed
Address feedback
1 parent e5d2e7b commit c0d44db

5 files changed

Lines changed: 30 additions & 60 deletions

File tree

packages/module/src/MessageBox/JumpButton.scss

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -44,28 +44,6 @@
4444
inset-block-end: var(--pf-t--global--spacer--md) !important;
4545
}
4646

47-
// inclusively hidden: stay in tab order but reveal on keyboard focus
48-
// https://css-tricks.com/inclusively-hidden/
49-
&--hidden:not(:focus):not(:focus-visible):not(:active) {
50-
clip: rect(0 0 0 0);
51-
clip-path: inset(50%);
52-
height: 1px !important;
53-
overflow: hidden;
54-
white-space: nowrap;
55-
width: 1px !important;
56-
}
57-
58-
&--hidden:focus,
59-
&--hidden:focus-visible,
60-
&--hidden:active {
61-
clip: auto;
62-
clip-path: none;
63-
height: 2rem !important;
64-
overflow: visible;
65-
white-space: normal;
66-
width: 2rem !important;
67-
}
68-
6947
// allows for zoom conditions; try zooming to 200% to see
7048
@media screen and (max-height: 518px) {
7149
display: none;

packages/module/src/MessageBox/JumpButton.test.tsx

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,8 @@ describe('JumpButton', () => {
1818
await userEvent.click(screen.getByRole('button', { name: /Back to bottom/i }));
1919
expect(spy).toHaveBeenCalledTimes(1);
2020
});
21-
it('should remain in the DOM but visually hidden when isHidden is used', () => {
21+
it('should be hidden if isHidden prop is used', () => {
2222
render(<JumpButton position="bottom" onClick={jest.fn()} isHidden />);
23-
const button = screen.getByRole('button', { name: /Back to bottom/i });
24-
expect(button).toHaveClass('pf-chatbot__jump--hidden');
25-
});
26-
27-
it('should become visible when focused while hidden', () => {
28-
render(<JumpButton position="bottom" onClick={jest.fn()} isHidden />);
29-
const button = screen.getByRole('button', { name: /Back to bottom/i });
30-
button.focus();
31-
expect(button).toHaveFocus();
32-
expect(button).toHaveClass('pf-chatbot__jump--hidden');
23+
expect(screen.queryByRole('button', { name: /Back to bottom/i })).toBeFalsy();
3324
});
3425
});

packages/module/src/MessageBox/JumpButton.tsx

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,25 +28,26 @@ const JumpButton: FunctionComponent<JumpButtonProps> = ({
2828
onClick,
2929
jumpButtonProps,
3030
jumpButtonTooltipProps
31-
}: JumpButtonProps) => (
32-
<Tooltip
33-
id={`pf-chatbot__tooltip--jump-${position}`}
34-
content={`Back to ${position}`}
35-
position="top"
36-
{...jumpButtonTooltipProps}
37-
>
38-
<Button
39-
variant="plain"
40-
className={`pf-chatbot__jump pf-chatbot__jump--${position} ${isHidden ? 'pf-chatbot__jump--hidden' : ''}`}
41-
aria-label={`Back to ${position}`}
42-
onClick={onClick}
43-
{...jumpButtonProps}
31+
}: JumpButtonProps) =>
32+
isHidden ? null : (
33+
<Tooltip
34+
id={`pf-chatbot__tooltip--jump-${position}`}
35+
content={`Back to ${position}`}
36+
position="top"
37+
{...jumpButtonTooltipProps}
4438
>
45-
<Icon iconSize="lg" isInline>
46-
{position === 'top' ? <ArrowUpIcon /> : <ArrowDownIcon />}
47-
</Icon>
48-
</Button>
49-
</Tooltip>
50-
);
39+
<Button
40+
variant="plain"
41+
className={`pf-chatbot__jump pf-chatbot__jump--${position}`}
42+
aria-label={`Back to ${position}`}
43+
onClick={onClick}
44+
{...jumpButtonProps}
45+
>
46+
<Icon iconSize="lg" isInline>
47+
{position === 'top' ? <ArrowUpIcon /> : <ArrowDownIcon />}
48+
</Icon>
49+
</Button>
50+
</Tooltip>
51+
);
5152

5253
export default JumpButton;

packages/module/src/MessageBox/MessageBox.test.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -378,8 +378,8 @@ describe('MessageBox', () => {
378378
});
379379

380380
await waitFor(() => {
381-
expect(screen.getByRole('button', { name: /Back to top/i })).toHaveClass('pf-chatbot__jump--hidden');
382-
expect(screen.getByRole('button', { name: /Back to bottom/i })).not.toHaveClass('pf-chatbot__jump--hidden');
381+
expect(screen.queryByRole('button', { name: /Back to top/i })).toBeFalsy();
382+
expect(screen.getByRole('button', { name: /Back to bottom/i })).toBeTruthy();
383383
});
384384
});
385385

@@ -400,8 +400,8 @@ describe('MessageBox', () => {
400400
});
401401

402402
await waitFor(() => {
403-
expect(screen.getByRole('button', { name: /Back to top/i })).not.toHaveClass('pf-chatbot__jump--hidden');
404-
expect(screen.getByRole('button', { name: /Back to bottom/i })).toHaveClass('pf-chatbot__jump--hidden');
403+
expect(screen.getByRole('button', { name: /Back to top/i })).toBeTruthy();
404+
expect(screen.queryByRole('button', { name: /Back to bottom/i })).toBeFalsy();
405405
});
406406
});
407407
});

packages/module/src/MessageBox/MessageBox.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,10 @@ export const MessageBox = forwardRef(
166166
);
167167
setIsOverflowing(metrics.isOverflowing);
168168

169+
if (isAtBottom) {
170+
programmaticScrollRef.current = false;
171+
}
172+
169173
if (!enableSmartScroll || scrollQueued.current) {
170174
return;
171175
}
@@ -178,10 +182,6 @@ export const MessageBox = forwardRef(
178182
clearTimeout(debounceTimeout.current);
179183
}
180184

181-
if (isAtBottom) {
182-
programmaticScrollRef.current = false;
183-
}
184-
185185
if (!isAtBottom && !metrics.nearBottom && !pauseAutoScrollRef.current && !programmaticScrollRef.current) {
186186
setAutoScroll(false);
187187
}

0 commit comments

Comments
 (0)