Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add rounded corners to all components #5938

Merged
merged 1 commit into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions src/annotator/components/AdderToolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ function NumberIcon({ badgeCount }: { badgeCount: number }) {
return (
<span
className={classnames(
'rounded-[4px] px-1 py-0.5',
'rounded px-1 py-0.5',
// The background color is inherited from the current text color in
// the containing button and will vary depending on hover state.
'bg-current',
Expand Down Expand Up @@ -238,8 +238,8 @@ export default function AdderToolbar({
// default border values from Tailwind and have to be explicit about all
// border attributes.
'border border-solid border-grey-3',
'absolute select-none bg-white rounded-[4px] shadow-adder-toolbar',
// Start at a very low opacity as we're going to fade in in the animation
'absolute select-none bg-white rounded shadow-adder-toolbar',
// Start at a very low opacity as we're going to fade-in in the animation
'opacity-5',
{
'animate-adder-pop-up': arrowDirection === 'up' && isVisible,
Expand Down
5 changes: 3 additions & 2 deletions src/annotator/components/ClusterToolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ function ClusterStyleControl({
<div className="space-y-2">
<div className="flex items-center gap-x-2 text-annotator-base">
<div
className="grow text-color-text px-2 py-1 rounded-[4px]"
className="grow text-color-text px-2 py-1 rounded"
style={{
backgroundColor: highlightStyles[appliedStyleName].color,
}}
Expand Down Expand Up @@ -130,9 +130,10 @@ export default function ClusterToolbar({
}

return (
<Card>
<Card classes="overflow-hidden">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

<div className="flex flex-col text-annotator-base text-color-text">
<Button
classes="rounded-none"
data-testid="control-toggle-button"
onClick={() => setOpen(!isOpen)}
title={isOpen ? 'Hide highlight settings' : 'Show highlight settings'}
Expand Down
4 changes: 2 additions & 2 deletions src/annotator/components/Toolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function ToolbarButton({ icon: Icon, ...buttonProps }: ToolbarButtonProps) {
return (
<Button
classes={classnames(
'justify-center rounded-[4px]',
'justify-center rounded',
'w-[30px] h-[30px]',
'shadow border bg-white text-grey-6 hover:text-grey-9',
)}
Expand Down Expand Up @@ -162,7 +162,7 @@ export default function Toolbar({
classes={classnames(
'transition-colors focus-visible-ring ring-inset',
// Height and width to align with the sidebar's top bar
'h-[40px] w-[33px] pl-[6px]',
'h-[40px] w-[33px] pl-[6px] rounded-bl',
'bg-white text-grey-5 hover:text-grey-9',
// Turn on left and bottom borders to continue the
// border of the sidebar's top bar
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ function AnnotationPublishControl({
<div
className={classnames(
// Round the right side of this menu-open button only
'flex flex-row rounded-r-sm bg-grey-7 hover:bg-grey-8',
'flex flex-row rounded-r bg-grey-7 hover:bg-grey-8',
)}
style={buttonStyle}
>
Expand Down
4 changes: 2 additions & 2 deletions src/sidebar/components/Annotation/AnnotationShareControl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ function AnnotationShareControl({
<div
// Position this Card above its IconButton. Account for larger
// IconButtons in touch interfaces
className="absolute bottom-8 right-1 touch:bottom-touch-minimum"
className="absolute bottom-8 right-0 touch:bottom-touch-minimum"
>
<Card
classes={classnames(
Expand Down Expand Up @@ -195,7 +195,7 @@ function AnnotationShareControl({
{showShareLinks && <ShareLinks shareURI={shareUri} />}
<MenuArrow
direction="down"
classes="bottom-[-8px] right-1 touch:right-[9px]"
classes="bottom-[-8px] right-2 touch:right-[9px]"
/>
</Card>
</div>
Expand Down
4 changes: 2 additions & 2 deletions src/sidebar/components/AutocompleteList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,11 @@ export default function AutocompleteList<Item>({
)}
data-testid="autocomplete-list-container"
>
<Card width="auto">
<Card width="auto" classes="overflow-hidden">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

<ul tabIndex={-1} aria-label="Suggestions" role="listbox" {...props}>
{items}
</ul>
<MenuArrow direction="up" classes="top-[-8px] left-[3px]" />
<MenuArrow direction="up" classes="top-[-8px] left-2" />
</Card>
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion src/sidebar/components/MarkdownEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ function Toolbar({ isPreviewing, onCommand, onTogglePreview }: ToolbarProps) {
className={classnames(
// Allow buttons to wrap to second line if necessary.
'flex flex-wrap w-full items-center',
'p-1 border-x border-t rounded-t-[4px] bg-white',
'p-1 border-x border-t rounded-t bg-white',
// For touch interfaces, allow height to scale to larger button targets.
// Don't wrap buttons but instead scroll horizontally. Add bottom
// padding to provide some space for scrollbar.
Expand Down
10 changes: 6 additions & 4 deletions src/sidebar/components/Menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -232,19 +232,21 @@ export default function Menu({
direction="up"
classes={classnames(
// Position menu-arrow caret near bottom right of menu label/toggle control
'right-0 top-[calc(100%-3px)] w-[15px]',
'right-1 top-[calc(100%-3px)] w-[15px]',
arrowClass,
)}
/>
<div
className={classnames(
'focus-visible-ring',
// Position menu content near bottom of menu label/toggle control
'absolute top-[calc(100%+5px)] z-1 border shadow',
'bg-white text-md',
'absolute top-[calc(100%+5px)] z-1',
'border shadow rounded-lg overflow-hidden bg-white text-md',
{
'left-0': align === 'left',
'right-0': align === 'right',
// Adding negative right distance so that the menu arrow does
// not overlap with the top-right corner when it's rounded
'-right-1': align === 'right',
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering whether we should remove the up arrow for menus where the up-arrow was previously aligned with either the right or left edge of its toggle button. This seems to be the pattern that several web properties (eg. GitHub) use.

},
contentClass,
)}
Expand Down
4 changes: 2 additions & 2 deletions src/sidebar/components/MenuItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ export default function MenuItem({
const wrapperClasses = classnames(
'focus-visible-ring ring-inset',
'w-full min-w-[150px] flex items-center select-none',
'border-b',
'border-b rounded-none cursor-pointer',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This addresses inconsistencies in which link menu items were being rendered with rounded corners on hover, while non-links were not rounded.

I took the opportunity to address other inconsistencies between link vs non-link menu items, like hover color and cursor type.

// Set this container as a "group" so that children may style based on its
// layout state.
// See https://tailwindcss.com/docs/hover-focus-and-other-states#styling-based-on-parent-state
Expand All @@ -263,7 +263,7 @@ export default function MenuItem({
'border-b-grey-3': isExpanded,
'border-b-transparent': !isExpanded,
'text-color-text-light': isDisabled,
'text-color-text': !isDisabled,
'text-color-text hover:text-color-text': !isDisabled,
},
);

Expand Down
2 changes: 1 addition & 1 deletion src/sidebar/components/PaginationNavigation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ function NavigationButton({ ...buttonProps }: NavigationButtonProps) {
<Button
classes={classnames(
'px-3.5 py-2.5 gap-x-1',
'font-semibold',
'font-semibold rounded',
// These colors are the same as the "dark" variant of IconButton
'text-grey-7 bg-grey-2 enabled:hover:text-grey-9 enabled:hover:bg-grey-3',
'disabled:text-grey-5 aria-pressed:bg-grey-3 aria-expanded:bg-grey-3',
Expand Down
8 changes: 4 additions & 4 deletions src/sidebar/components/ProfileView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ function ToastBadge({
return (
<div
className={classnames(
'flex items-center gap-x-1 py-1 px-2 rounded-[4px]',
'flex items-center gap-x-1 py-1 px-2 rounded',
'bg-green-success/10 animate-pulse-fade-out',
classes,
)}
Expand Down Expand Up @@ -97,13 +97,13 @@ export default function ProfileView() {

// Render save-success message after each successful save, but do not render
// it when a "request is in flight". This removal and re-adding across a
// sequence of saves ensures that the browser sees the message as newly- added
// to the accessiblity DOM and screen readers should announce it at the
// sequence of saves ensures that the browser sees the message as newly-added
// to the accessibility DOM and screen readers should announce it at the
// appropriate times.
const withSaveMessage = saveCount > 0 && !loading;

return (
<Card data-testid="profile-container">
<Card data-testid="profile-container" classes="overflow-hidden">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

<div
className={classnames(
// Ensure there is enough height to clear both the heading text and the
Expand Down
9 changes: 6 additions & 3 deletions src/sidebar/components/ShareDialog/ShareDialog.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Card, Tab } from '@hypothesis/frontend-shared';
import classnames from 'classnames';
import { useState } from 'preact/hooks';

import { useSidebarStore } from '../../store';
Expand Down Expand Up @@ -34,10 +35,12 @@ export default function ShareDialog({
const panelTitle = `Share Annotations in ${groupName}`;

const tabbedDialog = exportTab || importTab;
// Determine initial selected tab, based on the first tab that will be displayed
const initialTab = shareTab ? 'share' : exportTab ? 'export' : 'import';
const [selectedTab, setSelectedTab] = useState<'share' | 'export' | 'import'>(
// Determine initial selected tab, based on the first tab that will be displayed
shareTab ? 'share' : exportTab ? 'export' : 'import',
initialTab,
);
const isFirstTabSelected = tabbedDialog && selectedTab === initialTab;

return (
<SidebarPanel
Expand Down Expand Up @@ -85,7 +88,7 @@ export default function ShareDialog({
</Tab>
)}
</TabHeader>
<Card>
<Card classes={classnames({ 'rounded-tl-none': isFirstTabSelected })}>
<TabPanel
id="share-panel"
active={selectedTab === 'share'}
Expand Down
1 change: 1 addition & 0 deletions src/sidebar/components/SidebarPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export default function SidebarPanel({
onClose={closePanel}
transitionComponent={Slider}
variant={variant}
scrollable={false}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to check why, but making the Dialog scrollable messes with the bottom corners and makes them non-rounded.

This specific Dialog does not have a fixed height, so the scroll does not add any value here and we can just disable it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

>
{children}
</Dialog>
Expand Down
2 changes: 1 addition & 1 deletion src/sidebar/components/SortMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export default function SortMenu() {
);

return (
<div className="SortMenu">
<div data-component="SortMenu">
<Menu
label={menuLabel}
title={`Sort by ${sortKey}`}
Expand Down
2 changes: 1 addition & 1 deletion src/sidebar/components/test/Menu-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ describe('Menu', () => {
assert.isTrue(wrapper.find(contentSelector).hasClass('left-0'));

wrapper.setProps({ align: 'right' });
assert.isTrue(wrapper.find(contentSelector).hasClass('right-0'));
assert.isTrue(wrapper.find(contentSelector).hasClass('-right-1'));
});

it('applies custom content class', () => {
Expand Down
10 changes: 10 additions & 0 deletions tailwind-annotator.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,14 @@ export default {
// This module references `sidebar-frame` and related classes
'./src/annotator/sidebar.{js,ts,tsx}',
],
theme: {
extend: {
borderRadius: {
// Equivalent to tailwind defaults, but overriding values from frontend-shared preset
// Once the preset stops defining borderRadius, we can remove this
DEFAULT: '0.25rem',
lg: '0.5rem',
},
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes annotator components be always rounded.

This was their behavior already, but it was set via hardcoded border radius rounded-[4px].

Those classes have been replaced by rounded, and this config ensures they keep looking the same regardless the feature being enabled or not, while it becomes ready for the future, when we just use tailwind defaults.

};
4 changes: 0 additions & 4 deletions tailwind.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ export default {
'pulse-fade-out': 'pulse-fade-out 5s ease-in-out forwards',
'slide-in-from-right': 'slide-in-from-right 0.3s forwards ease-in-out',
},
borderRadius: {
// Equivalent to tailwind default `rounded-sm` size
DEFAULT: '0.125rem',
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We now use the config defined in frontend-shared's tailwind preset.

boxShadow: {
'adder-toolbar': '0px 2px 10px 0px rgba(0, 0, 0, 0.25)',
focus: `0 0 0 2px ${focusBlue}`,
Expand Down