Skip to content

Commit 5befd55

Browse files
evanpurkhiserevanpurkhiser
authored
ref(ui): Improve uage of BookmarkStar (#32553)
1 parent 170de54 commit 5befd55

File tree

8 files changed

+165
-206
lines changed

8 files changed

+165
-206
lines changed

‎static/app/components/organizations/pageFilterRow.tsx‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ type CheckboxRenderOptions = {
1212
checked?: boolean;
1313
};
1414

15-
type Props = {
15+
interface Props extends React.HTMLAttributes<HTMLDivElement> {
1616
checked: boolean;
1717
children: React.ReactNode;
1818
onCheckClick: (event: React.MouseEvent) => void;
@@ -23,7 +23,7 @@ type Props = {
2323
* prop along with the `checked` boolean.
2424
*/
2525
renderCheckbox?: (options: CheckboxRenderOptions) => React.ReactNode;
26-
};
26+
}
2727

2828
function PageFilterRow({
2929
checked,
@@ -57,27 +57,26 @@ function PageFilterRow({
5757
const MultiselectCheckbox = styled(CheckboxFancy)`
5858
position: relative;
5959
margin: 0 ${space(1)};
60+
margin-right: ${space(0.75)};
6061
6162
/* Make the hitbox of the checkbox a bit larger */
6263
&:after {
6364
content: '';
6465
position: absolute;
6566
top: -${space(2)};
66-
right: -${space(2)};
67+
right: -${space(1.5)};
6768
bottom: -${space(2)};
6869
left: -${space(2)};
6970
}
7071
`;
7172

7273
const Container = styled('div')<{isChecked: boolean}>`
73-
display: flex;
74+
display: grid;
75+
grid-template-columns: 1fr max-content;
7476
align-items: center;
75-
justify-content: space-between;
7677
font-size: ${p => p.theme.fontSizeMedium};
7778
font-weight: 400;
7879
padding-left: ${space(0.5)};
79-
height: ${p => p.theme.headerSelectorRowHeight}px;
80-
flex-shrink: 0;
8180
8281
${MultiselectCheckbox} {
8382
opacity: ${p => (p.isChecked ? 1 : 0.33)};

‎static/app/components/organizations/projectSelector/selectorItem.tsx‎

Lines changed: 52 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@ import styled from '@emotion/styled';
33

44
import Feature from 'sentry/components/acl/feature';
55
import FeatureDisabled from 'sentry/components/acl/featureDisabled';
6+
import Button from 'sentry/components/button';
67
import Highlight from 'sentry/components/highlight';
78
import {Hovercard} from 'sentry/components/hovercard';
89
import IdBadge from 'sentry/components/idBadge';
9-
import Link from 'sentry/components/links/link';
1010
import PageFilterRow from 'sentry/components/organizations/pageFilterRow';
1111
import BookmarkStar from 'sentry/components/projects/bookmarkStar';
1212
import {IconOpen, IconSettings} from 'sentry/icons';
@@ -71,98 +71,76 @@ function ProjectSelectorItem({
7171
};
7272

7373
return (
74-
<BadgeAndActionsWrapper>
75-
<PageFilterRow
76-
checked={isChecked}
77-
onCheckClick={handleClick}
78-
multi={multi}
79-
renderCheckbox={({checkbox}) => (
80-
<Feature
81-
features={['organizations:global-views']}
82-
hookName="feature-disabled:project-selector-checkbox"
83-
renderDisabled={renderDisabledCheckbox}
84-
>
85-
{checkbox}
86-
</Feature>
87-
)}
88-
>
89-
<BadgeWrapper isMulti={multi}>
90-
<IdBadge
91-
project={project}
92-
avatarSize={16}
93-
displayName={<Highlight text={inputValue}>{project.slug}</Highlight>}
94-
avatarProps={{consistentWidth: true}}
95-
disableLink
96-
/>
97-
</BadgeWrapper>
98-
<StyledBookmarkStar
74+
<ProjectFilterRow
75+
checked={isChecked}
76+
onCheckClick={handleClick}
77+
multi={multi}
78+
renderCheckbox={({checkbox}) => (
79+
<Feature
80+
features={['organizations:global-views']}
81+
hookName="feature-disabled:project-selector-checkbox"
82+
renderDisabled={renderDisabledCheckbox}
83+
>
84+
{checkbox}
85+
</Feature>
86+
)}
87+
>
88+
<BadgeWrapper>
89+
<IdBadge
9990
project={project}
100-
organization={organization}
101-
onToggle={handleBookmarkToggle}
91+
avatarSize={16}
92+
displayName={<Highlight text={inputValue}>{project.slug}</Highlight>}
93+
avatarProps={{consistentWidth: true}}
94+
disableLink
10295
/>
103-
<StyledLink
104-
to={`/organizations/${organization.slug}/projects/${project.slug}/?project=${project.id}`}
105-
onClick={e => e.stopPropagation()}
106-
>
107-
<IconOpen />
108-
</StyledLink>
109-
110-
<StyledLink
111-
to={`/settings/${organization.slug}/${project.slug}/`}
112-
onClick={e => e.stopPropagation()}
113-
>
114-
<IconSettings />
115-
</StyledLink>
116-
</PageFilterRow>
117-
</BadgeAndActionsWrapper>
96+
</BadgeWrapper>
97+
<ActionBookmark
98+
project={project}
99+
organization={organization}
100+
onToggle={handleBookmarkToggle}
101+
/>
102+
<ActionButton
103+
to={`/organizations/${organization.slug}/projects/${project.slug}/?project=${project.id}`}
104+
size="zero"
105+
priority="link"
106+
aria-label="Project Details"
107+
icon={<IconOpen />}
108+
/>
109+
<ActionButton
110+
to={`/settings/${organization.slug}/${project.slug}/`}
111+
size="zero"
112+
priority="link"
113+
aria-label="Project Settings"
114+
icon={<IconSettings />}
115+
/>
116+
</ProjectFilterRow>
118117
);
119118
}
120119

121120
export default ProjectSelectorItem;
122121

123-
const StyledBookmarkStar = styled(BookmarkStar)`
124-
padding: ${space(1)} ${space(0.5)};
125-
box-sizing: content-box;
126-
opacity: ${p => (p.project.isBookmarked ? 1 : 0.33)};
127-
transition: 0.5s opacity ease-out;
128-
display: block;
129-
width: 14px;
130-
height: 14px;
131-
margin-top: -${space(0.25)}; /* trivial alignment bump */
132-
`;
133-
134-
const BadgeWrapper = styled('div')<{isMulti: boolean}>`
122+
const BadgeWrapper = styled('div')`
135123
display: flex;
136124
flex: 1;
137-
${p => !p.isMulti && 'flex: 1'};
138125
white-space: nowrap;
139126
overflow: hidden;
140127
`;
141128

142-
const StyledLink = styled(Link)`
143-
color: ${p => p.theme.gray300};
144-
display: flex;
145-
align-items: center;
146-
justify-content: space-between;
129+
const ActionButton = styled(Button)`
130+
color: ${p => p.theme.subText};
147131
padding: ${space(1)} ${space(0.25)} ${space(1)} ${space(1)};
148132
opacity: 0.33;
149-
transition: 0.5s opacity ease-out;
150133
:hover {
151134
color: ${p => p.theme.textColor};
152135
}
153136
`;
154137

155-
const BadgeAndActionsWrapper = styled('div')`
156-
position: relative;
157-
border-style: solid;
158-
border-width: 1px 0;
159-
border-color: transparent;
160-
:hover {
161-
${StyledBookmarkStar} {
162-
opacity: 1;
163-
}
164-
${StyledLink} {
165-
opacity: 1;
166-
}
138+
const ActionBookmark = styled(BookmarkStar)`
139+
${p => !p.project.isBookmarked && 'opacity: 0.33'};
140+
`;
141+
142+
const ProjectFilterRow = styled(PageFilterRow)`
143+
:hover ${ActionButton}, :hover ${ActionBookmark} {
144+
opacity: 1;
167145
}
168146
`;

‎static/app/components/projects/bookmarkStar.tsx‎

Lines changed: 51 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
import * as React from 'react';
21
import styled from '@emotion/styled';
32

43
import {addErrorMessage} from 'sentry/actionCreators/indicator';
54
import {update} from 'sentry/actionCreators/projects';
5+
import Button from 'sentry/components/button';
66
import {IconStar} from 'sentry/icons';
77
import {t} from 'sentry/locale';
88
import {Organization, Project} from 'sentry/types';
@@ -13,59 +13,65 @@ type Props = {
1313
organization: Organization;
1414
project: Project;
1515
className?: string;
16-
/* used to override when under local state */
16+
/**
17+
* Allows the bookmarked state of the project to be overriden. Useful for
18+
* optimistic updates.
19+
*/
1720
isBookmarked?: boolean;
1821
onToggle?: (isBookmarked: boolean) => void;
1922
};
2023

21-
const BookmarkStar = ({
22-
isBookmarked: isBookmarkedProp,
23-
className,
24-
organization,
25-
project,
26-
onToggle,
27-
}: Props) => {
28-
const api = useApi();
24+
const BookmarkStar = styled(
25+
({
26+
isBookmarked: isBookmarkedProp,
27+
className,
28+
organization,
29+
project,
30+
onToggle,
31+
}: Props) => {
32+
const api = useApi();
2933

30-
const isBookmarked = defined(isBookmarkedProp)
31-
? isBookmarkedProp
32-
: project.isBookmarked;
34+
const isBookmarked = defined(isBookmarkedProp)
35+
? isBookmarkedProp
36+
: project.isBookmarked;
3337

34-
const toggleProjectBookmark = (event: React.MouseEvent) => {
35-
update(api, {
36-
orgId: organization.slug,
37-
projectId: project.slug,
38-
data: {isBookmarked: !isBookmarked},
39-
}).catch(() => {
40-
addErrorMessage(t('Unable to toggle bookmark for %s', project.slug));
41-
});
38+
const toggleProjectBookmark = (event: React.MouseEvent) => {
39+
update(api, {
40+
orgId: organization.slug,
41+
projectId: project.slug,
42+
data: {isBookmarked: !isBookmarked},
43+
}).catch(() =>
44+
addErrorMessage(t('Unable to toggle bookmark for %s', project.slug))
45+
);
4246

43-
// needed to dismiss tooltip
44-
(document.activeElement as HTMLElement).blur();
47+
// prevent dropdowns from closing
48+
event.stopPropagation();
4549

46-
// prevent dropdowns from closing
47-
event.stopPropagation();
50+
onToggle?.(!isBookmarked);
51+
};
4852

49-
if (onToggle) {
50-
onToggle(!isBookmarked);
51-
}
52-
};
53-
54-
return (
55-
<Star
56-
isBookmarked={isBookmarked}
57-
isSolid={isBookmarked}
58-
onClick={toggleProjectBookmark}
59-
className={className}
60-
/>
61-
);
62-
};
63-
64-
const Star = styled(IconStar, {shouldForwardProp: p => p !== 'isBookmarked'})<{
65-
isBookmarked: boolean;
66-
}>`
67-
color: ${p => (p.isBookmarked ? p.theme.yellow300 : p.theme.gray200)};
68-
cursor: pointer;
53+
return (
54+
<Button
55+
aria-label="Bookmark Project"
56+
aria-pressed={isBookmarked}
57+
onClick={toggleProjectBookmark}
58+
size="zero"
59+
priority="link"
60+
className={className}
61+
icon={
62+
<IconStar
63+
color={isBookmarked ? 'yellow300' : 'subText'}
64+
isSolid={isBookmarked}
65+
/>
66+
}
67+
/>
68+
);
69+
}
70+
)`
71+
&:hover svg {
72+
fill: ${p =>
73+
p.project.isBookmarked || p.isBookmarked ? p.theme.yellow400 : p.theme.textColor};
74+
}
6975
`;
7076

7177
export default BookmarkStar;

0 commit comments

Comments
 (0)