Skip to content

Commit

Permalink
Fix top bar toggle buttons extending outside top bar on touch devices
Browse files Browse the repository at this point in the history
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 #6131.
  • Loading branch information
robertknight committed Jan 24, 2024
1 parent 63033b1 commit a228947
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 14 deletions.
27 changes: 23 additions & 4 deletions src/sidebar/components/TopBar.tsx
@@ -1,3 +1,4 @@
import type { IconButtonProps } from '@hypothesis/frontend-shared';
import { LinkButton, HelpIcon, ShareIcon } from '@hypothesis/frontend-shared';
import classnames from 'classnames';

Expand Down Expand Up @@ -34,6 +35,26 @@ export type TopBarProps = {
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
* to switch groups, view account information, sort/filter annotations etc.
Expand Down Expand Up @@ -105,23 +126,21 @@ function TopBar({
)}
{searchPanelEnabled && <SearchIconButton />}
<SortMenu />
<PressableIconButton
<TopBarToggleButton
icon={ShareIcon}
expanded={isAnnotationsPanelOpen}
pressed={isAnnotationsPanelOpen}
onClick={toggleSharePanel}
size="xs"
title="Share annotations on this page"
data-testid="share-icon-button"
/>
</>
)}
<PressableIconButton
<TopBarToggleButton
icon={HelpIcon}
expanded={isHelpPanelOpen}
pressed={isHelpPanelOpen}
onClick={requestHelp}
size="xs"
title="Help"
data-testid="help-icon-button"
/>
Expand Down
12 changes: 2 additions & 10 deletions src/sidebar/components/search/SearchIconButton.tsx
Expand Up @@ -5,7 +5,7 @@ import { useShortcut } from '../../../shared/shortcut';
import { isMacOS } from '../../../shared/user-agent';
import type { SidebarStore } from '../../store';
import { useSidebarStore } from '../../store';
import PressableIconButton from '../PressableIconButton';
import { TopBarToggleButton } from '../TopBar';

/**
* Respond to keydown events on the document (shortcut keys):
Expand Down Expand Up @@ -61,20 +61,12 @@ export default function SearchIconButton() {
<>
{isLoading && <Spinner />}
{!isLoading && (
<PressableIconButton
<TopBarToggleButton
icon={SearchIcon}
expanded={isSearchPanelOpen}
pressed={isSearchPanelOpen}
onClick={toggleSearchPanel}
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"
/>
)}
</>
Expand Down

0 comments on commit a228947

Please sign in to comment.