Skip to content

Commit 5533639

Browse files
siddharthkpsiddharthkp
authored
UI improvements for comments (codesandbox#3658)
* add space around empty message * fix width of list with overflow:auto * match spacing with design * fix drag handle for popup * fix if check for element margin * fix height of scrollable list * add shadow for comment box * use text props, align comment header * fix spacing in comment and replies * add makeshift overflow for popup * add check for currentComment * remove comment from the wrong place
1 parent 22f2f08 commit 5533639

File tree

8 files changed

+152
-116
lines changed

8 files changed

+152
-116
lines changed

‎packages/app/src/app/pages/Sandbox/Editor/Workspace/screens/Comments/AddComment.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,11 @@ export const AddComment: React.FC = () => {
2828
paddingX={2}
2929
paddingY={4}
3030
css={css({
31-
boxShadow: '2',
3231
borderTop: '1px solid',
3332
borderColor: 'sideBar.border',
33+
// super custom shadow, TODO: check if this works in other themes
34+
boxShadow:
35+
'0px -4px 8px rgba(21, 21, 21, 0.4), 0px -8px 8px rgba(21, 21, 21, 0.4)',
3436
})}
3537
>
3638
<form onSubmit={onSubmit}>

‎packages/app/src/app/pages/Sandbox/Editor/Workspace/screens/Comments/Comment.tsx‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export const Comment = React.memo(({ comment }: any) => {
3232
return (
3333
<ListAction
3434
key={comment.id}
35-
paddingTop={5}
35+
paddingTop={4}
3636
css={css({
3737
display: 'block',
3838
color: 'inherit',
@@ -51,13 +51,14 @@ export const Comment = React.memo(({ comment }: any) => {
5151
<Avatar user={comment.originalMessage.author} />
5252
<Stack direction="vertical" justify="center">
5353
<Link
54+
size={3}
55+
weight="bold"
5456
href={`/u/${comment.originalMessage.author.username}`}
5557
variant="body"
56-
css={{ fontWeight: 'bold', display: 'block' }}
5758
>
5859
{comment.originalMessage.author.username}
5960
</Link>
60-
<Text size={12} variant="muted">
61+
<Text size={2} variant="muted">
6162
{formatDistance(new Date(comment.insertedAt), new Date(), {
6263
addSuffix: true,
6364
})}
@@ -100,7 +101,7 @@ export const Comment = React.memo(({ comment }: any) => {
100101
as="p"
101102
marginY={0}
102103
marginRight={2 /** Adjust for the missing margin in ListAction */}
103-
paddingBottom={5}
104+
paddingBottom={6 /** Use padding instead of margin for inset border */}
104105
css={css({
105106
borderBottom: '1px solid',
106107
borderColor: 'sideBar.border',

‎packages/app/src/app/pages/Sandbox/Editor/Workspace/screens/Comments/Dialog/Comment.tsx‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@ export const Comment = ({ source }) => {
2828

2929
return (
3030
<Element
31-
paddingX={4}
32-
paddingBottom={6}
3331
css={css({
3432
'ul, ol': {
3533
paddingLeft: 0,

‎packages/app/src/app/pages/Sandbox/Editor/Workspace/screens/Comments/Dialog/Reply.tsx‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,27 +30,20 @@ export const Reply = ({
3030
const { state, actions } = useOvermind();
3131
return (
3232
<>
33-
<Element
34-
key={id}
35-
paddingX={4}
36-
paddingTop={6}
37-
css={css({
38-
borderTop: '1px solid',
39-
borderColor: 'sideBar.border',
40-
})}
41-
>
33+
<Element key={id} marginLeft={4} marginRight={2} paddingTop={6}>
4234
<Stack align="flex-start" justify="space-between" marginBottom={4}>
4335
<Stack gap={2} align="center">
4436
<Avatar user={author} />
45-
<Stack direction="vertical" justify="center">
37+
<Stack direction="vertical" justify="center" gap={1}>
4638
<Link
39+
size={3}
40+
weight="bold"
4741
href={`/u/${author.username}`}
4842
variant="body"
49-
css={{ fontWeight: 'bold', display: 'block' }}
5043
>
5144
{author.username}
5245
</Link>
53-
<Text size={12} variant="muted">
46+
<Text size={2} variant="muted">
5447
{formatDistance(new Date(insertedAt), new Date(), {
5548
addSuffix: true,
5649
})}
@@ -79,7 +72,18 @@ export const Reply = ({
7972
)}
8073
</Stack>
8174
</Element>
82-
<Comment source={content} />
75+
<Element
76+
as="p"
77+
marginY={0}
78+
marginX={4}
79+
paddingBottom={6}
80+
css={css({
81+
borderBottom: '1px solid',
82+
borderColor: 'sideBar.border',
83+
})}
84+
>
85+
<Comment source={content} />
86+
</Element>
8387
</>
8488
);
8589
};

‎packages/app/src/app/pages/Sandbox/Editor/Workspace/screens/Comments/Dialog/index.tsx‎

Lines changed: 89 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -64,99 +64,112 @@ export const Dialog = props => {
6464
borderRadius: 4,
6565
width: 420,
6666
height: 'auto',
67+
maxHeight: '80vh',
68+
overflow: 'auto',
69+
fontFamily: 'Inter, sans-serif',
70+
boxShadow: 2,
6771
})}
6872
>
69-
<Stack direction="vertical">
70-
<Stack
71-
align="center"
72-
justify="space-between"
73-
padding={4}
74-
marginBottom={2}
75-
>
76-
<Text size={3} weight="bold">
77-
Comment
78-
</Text>
79-
<Stack align="center">
80-
<IconButton
81-
onClick={() =>
82-
actions.editor.updateComment({
83-
id: comment.id,
84-
data: { isResolved: !comment.isResolved },
85-
})
86-
}
87-
name="check"
88-
title="Resolve Comment"
89-
css={css({
90-
transition: 'color',
91-
transitionDuration: theme => theme.speeds[1],
92-
color: comment.isResolved ? 'green' : 'mutedForeground',
93-
})}
94-
/>
95-
<IconButton
96-
name="cross"
97-
size={3}
98-
title="Close comment dialog"
99-
onClick={closeDialog}
100-
/>{' '}
101-
</Stack>
73+
<Stack
74+
className="handle"
75+
css={{ cursor: 'move' }}
76+
align="center"
77+
justify="space-between"
78+
padding={4}
79+
paddingRight={2}
80+
marginBottom={2}
81+
>
82+
<Text size={3} weight="bold">
83+
Comment
84+
</Text>
85+
<Stack align="center">
86+
<IconButton
87+
onClick={() =>
88+
actions.editor.updateComment({
89+
id: comment.id,
90+
data: { isResolved: !comment.isResolved },
91+
})
92+
}
93+
name="check"
94+
size={4}
95+
title="Resolve Comment"
96+
css={css({
97+
transition: 'color',
98+
transitionDuration: theme => theme.speeds[1],
99+
color: comment.isResolved ? 'green' : 'mutedForeground',
100+
})}
101+
/>
102+
<IconButton
103+
name="cross"
104+
size={3}
105+
title="Close comment dialog"
106+
onClick={closeDialog}
107+
/>
102108
</Stack>
103-
<Stack
104-
className="handle"
105-
justify="space-between"
106-
padding={2}
107-
paddingLeft={4}
108-
css={{ cursor: 'move' }}
109-
marginBottom={4}
110-
>
111-
<Stack gap={2} align="center">
109+
</Stack>
110+
111+
{comment && (
112+
<>
113+
<Stack gap={2} align="center" paddingX={4} marginBottom={4}>
112114
<Avatar user={comment.originalMessage.author} />
113-
<Stack direction="vertical" justify="center">
115+
<Stack direction="vertical" justify="center" gap={1}>
114116
<Link
117+
size={3}
118+
weight="bold"
115119
href={`/u/${comment.originalMessage.author.username}`}
116120
variant="body"
117-
css={{ fontWeight: 'bold', display: 'block' }}
118121
>
119122
{comment.originalMessage.author.username}
120123
</Link>
121-
<Text size={12} variant="muted">
124+
<Text size={2} variant="muted">
122125
{formatDistance(new Date(comment.insertedAt), new Date(), {
123126
addSuffix: true,
124127
})}
125128
</Text>
126129
</Stack>
127130
</Stack>
128-
</Stack>
129-
{comment && (
130-
<>
131+
<Element
132+
as="p"
133+
marginY={0}
134+
marginX={4}
135+
paddingBottom={6}
136+
css={css({
137+
borderBottom: '1px solid',
138+
borderColor: 'sideBar.border',
139+
})}
140+
>
131141
<Comment source={comment.originalMessage.content} />
132-
{comment.replies.map(reply => (
133-
<Reply {...reply} commentId={comment.id} />
134-
))}
135-
<Element
136-
css={css({
137-
borderTop: '1px solid',
138-
borderColor: 'sideBar.border',
139-
})}
140-
>
141-
<Textarea
142-
autosize
143-
css={css({
144-
overflow: 'hidden',
145-
height: 50,
146-
border: 'none',
147-
display: 'block',
148-
})}
149-
value={value}
150-
onChange={e => setValue(e.target.value)}
151-
placeholder={comment ? 'Reply' : 'Write a comment...'}
152-
onKeyDown={event => {
153-
if (event.keyCode === ENTER && !event.shiftKey) onSubmit();
154-
}}
155-
/>
156-
</Element>
157-
</>
158-
)}
159-
</Stack>
142+
</Element>
143+
</>
144+
)}
145+
146+
{comment &&
147+
comment.replies.map(reply => (
148+
<Reply {...reply} commentId={comment.id} />
149+
))}
150+
151+
<Element
152+
css={css({
153+
borderTop: '1px solid',
154+
borderColor: 'sideBar.border',
155+
})}
156+
>
157+
<Textarea
158+
autosize
159+
css={css({
160+
overflow: 'hidden',
161+
162+
border: 'none',
163+
display: 'block',
164+
})}
165+
value={value}
166+
onChange={e => setValue(e.target.value)}
167+
placeholder={comment ? 'Reply' : 'Write a comment...'}
168+
onKeyDown={event => {
169+
if (event.keyCode === ENTER && !event.shiftKey) onSubmit();
170+
}}
171+
/>
172+
</Element>
160173
</Element>
161174
</Draggable>
162175
);

‎packages/app/src/app/pages/Sandbox/Editor/Workspace/screens/Comments/index.tsx‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,13 @@ export const Comments: React.FC = () => {
4040
selectedCommentsFilter === CommentsFilterOption.ALL;
4141

4242
const Empty = () => (
43-
<Stack direction="vertical" align="center" justify="center" gap={8}>
43+
<Stack
44+
direction="vertical"
45+
align="center"
46+
justify="center"
47+
gap={8}
48+
marginX={2}
49+
>
4450
<Icon
4551
name="comments"
4652
size={20}
@@ -97,7 +103,14 @@ export const Comments: React.FC = () => {
97103
</SidebarRow>
98104

99105
{currentComments.length ? (
100-
<List marginTop={4} css={{ height: '100%', overflow: 'scroll' }}>
106+
<List
107+
marginTop={4}
108+
css={{
109+
// stretch within container, leaving space for comment box
110+
height: 'calc(100% - 32px)',
111+
overflow: 'auto',
112+
}}
113+
>
101114
{currentComments.map(comment => (
102115
<Comment key={comment.id} comment={comment} />
103116
))}

‎packages/components/src/components/Element/index.tsx‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,26 @@ export interface IElementProps {
2222
export const Element = styled.div<IElementProps>(props =>
2323
css({
2424
boxSizing: 'border-box',
25-
margin: props.margin || null,
26-
marginX: props.marginX || null,
27-
marginY: props.marginY || null,
28-
marginBottom: props.marginBottom || null,
29-
marginTop: props.marginTop || null,
30-
marginLeft: props.marginLeft || null,
31-
marginRight: props.marginRight || null,
32-
padding: props.padding || null,
33-
paddingX: props.paddingX || null,
34-
paddingY: props.paddingY || null,
35-
paddingBottom: props.paddingBottom || null,
36-
paddingTop: props.paddingTop || null,
37-
paddingLeft: props.paddingLeft || null,
38-
paddingRight: props.paddingRight || null,
25+
margin: nullCheck(props.margin),
26+
marginX: nullCheck(props.marginX),
27+
marginY: nullCheck(props.marginY),
28+
marginBottom: nullCheck(props.marginBottom),
29+
marginTop: nullCheck(props.marginTop),
30+
marginLeft: nullCheck(props.marginLeft),
31+
marginRight: nullCheck(props.marginRight),
32+
padding: nullCheck(props.padding),
33+
paddingX: nullCheck(props.paddingX),
34+
paddingY: nullCheck(props.paddingY),
35+
paddingBottom: nullCheck(props.paddingBottom),
36+
paddingTop: nullCheck(props.paddingTop),
37+
paddingLeft: nullCheck(props.paddingLeft),
38+
paddingRight: nullCheck(props.paddingRight),
3939
...(props.css || {}),
4040
})
4141
);
42+
43+
const nullCheck = value => {
44+
// 0 is an allowed value, even though it's falsy
45+
if (typeof value !== 'undefined') return value;
46+
return null;
47+
};

‎packages/components/src/components/Link/index.tsx‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
import React from 'react';
22
import styled from 'styled-components';
33
import css from '@styled-system/css';
4-
import { Text } from '../Text';
4+
import { Text, ITextProps } from '../Text';
55

66
type LinkProps = React.AnchorHTMLAttributes<HTMLAnchorElement> &
7-
React.AnchorHTMLAttributes<HTMLSpanElement> & {
8-
variant?: 'body' | 'muted' | 'danger'; // same as Text
9-
};
7+
React.AnchorHTMLAttributes<HTMLSpanElement> &
8+
ITextProps;
109

1110
const LinkElement = styled(Text).attrs({ as: 'a' })<LinkProps>(
1211
css({

0 commit comments

Comments
 (0)