Commit a2289471 authored by Robert Knight's avatar Robert Knight

Fix top bar toggle buttons extending outside top bar on touch devices

There was already a solution for this issue implemented in `SearchIconButton`.
Extract that into a component that the other top bar buttons can use.

Part of https://github.com/hypothesis/client/issues/6131.
parent 63033b10
import type { IconButtonProps } from '@hypothesis/frontend-shared';
import { LinkButton, HelpIcon, ShareIcon } from '@hypothesis/frontend-shared'; import { LinkButton, HelpIcon, ShareIcon } from '@hypothesis/frontend-shared';
import classnames from 'classnames'; import classnames from 'classnames';
...@@ -34,6 +35,26 @@ export type TopBarProps = { ...@@ -34,6 +35,26 @@ export type TopBarProps = {
settings: SidebarSettings; settings: SidebarSettings;
}; };
/**
* Toggle button for the top bar, with a background to indicate its "pressed"
* state.
*/
export function TopBarToggleButton(buttonProps: IconButtonProps) {
return (
<PressableIconButton
// The containing form has a white background. The top bar is only
// 40px high. If we allow standard touch-minimum height here (44px),
// the visible white background exceeds the height of the top bar in
// touch contexts. Disable touch sizing via `size="custom"`, then
// add back the width rule and padding to keep horizontal spacing
// consistent.
size="custom"
classes="touch:min-w-touch-minimum p-1"
{...buttonProps}
/>
);
}
/** /**
* The toolbar which appears at the top of the sidebar providing actions * The toolbar which appears at the top of the sidebar providing actions
* to switch groups, view account information, sort/filter annotations etc. * to switch groups, view account information, sort/filter annotations etc.
...@@ -105,23 +126,21 @@ function TopBar({ ...@@ -105,23 +126,21 @@ function TopBar({
)} )}
{searchPanelEnabled && <SearchIconButton />} {searchPanelEnabled && <SearchIconButton />}
<SortMenu /> <SortMenu />
<PressableIconButton <TopBarToggleButton
icon={ShareIcon} icon={ShareIcon}
expanded={isAnnotationsPanelOpen} expanded={isAnnotationsPanelOpen}
pressed={isAnnotationsPanelOpen} pressed={isAnnotationsPanelOpen}
onClick={toggleSharePanel} onClick={toggleSharePanel}
size="xs"
title="Share annotations on this page" title="Share annotations on this page"
data-testid="share-icon-button" data-testid="share-icon-button"
/> />
</> </>
)} )}
<PressableIconButton <TopBarToggleButton
icon={HelpIcon} icon={HelpIcon}
expanded={isHelpPanelOpen} expanded={isHelpPanelOpen}
pressed={isHelpPanelOpen} pressed={isHelpPanelOpen}
onClick={requestHelp} onClick={requestHelp}
size="xs"
title="Help" title="Help"
data-testid="help-icon-button" data-testid="help-icon-button"
/> />
......
...@@ -5,7 +5,7 @@ import { useShortcut } from '../../../shared/shortcut'; ...@@ -5,7 +5,7 @@ import { useShortcut } from '../../../shared/shortcut';
import { isMacOS } from '../../../shared/user-agent'; import { isMacOS } from '../../../shared/user-agent';
import type { SidebarStore } from '../../store'; import type { SidebarStore } from '../../store';
import { useSidebarStore } from '../../store'; import { useSidebarStore } from '../../store';
import PressableIconButton from '../PressableIconButton'; import { TopBarToggleButton } from '../TopBar';
/** /**
* Respond to keydown events on the document (shortcut keys): * Respond to keydown events on the document (shortcut keys):
...@@ -61,20 +61,12 @@ export default function SearchIconButton() { ...@@ -61,20 +61,12 @@ export default function SearchIconButton() {
<> <>
{isLoading && <Spinner />} {isLoading && <Spinner />}
{!isLoading && ( {!isLoading && (
<PressableIconButton <TopBarToggleButton
icon={SearchIcon} icon={SearchIcon}
expanded={isSearchPanelOpen} expanded={isSearchPanelOpen}
pressed={isSearchPanelOpen} pressed={isSearchPanelOpen}
onClick={toggleSearchPanel} onClick={toggleSearchPanel}
title="Search annotations" title="Search annotations"
// The containing form has a white background. The top bar is only
// 40px high. If we allow standard touch-minimum height here (44px),
// the visible white background exceeds the height of the top bar in
// touch contexts. Disable touch sizing via `size="custom"`, then
// add back the width rule and padding to keep horizontal spacing
// consistent.
size="custom"
classes="touch:min-w-touch-minimum p-1"
/> />
)} )}
</> </>
......
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