-
Notifications
You must be signed in to change notification settings - Fork 408
✨(frontend) adjust doc emoji behaviour and introduce emoji picker #1381
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
base: main
Are you sure you want to change the base?
✨(frontend) adjust doc emoji behaviour and introduce emoji picker #1381
Conversation
0586dbb
to
6567267
Compare
9f4562a
to
54aeeaa
Compare
Thanks @olaurendeau ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works well ! Thank you 🎉
Mainly minor comments.
Under the Figma url, to be iso with what has made @rl-83 :
https://www.figma.com/design/qdCWR4tTUr7vQSecEjCyqO/DOCS?node-id=10856-18743&t=YHWKAzs0R27yHARe-0
src/frontend/apps/impress/src/features/docs/doc-editor/components/EmojiPicker.tsx
Outdated
Show resolved
Hide resolved
if (withOverlay) { | ||
return ( | ||
<> | ||
{/* Overlay transparent pour fermer en cliquant à l'extérieur */} | ||
<div | ||
style={{ | ||
position: 'fixed', | ||
top: 0, | ||
left: 0, | ||
width: '100vw', | ||
height: '100vh', | ||
zIndex: 999, | ||
backgroundColor: 'transparent', | ||
}} | ||
onClick={onClickOutside} | ||
/> | ||
{pickerContent} | ||
</> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary ? The picker has already onClickOutside
, without this part the picker seems to close correctly when click outside.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onClickOutside
is working fine for everything but the emoji button. There's maybe a better way to handle this issue ?
Example with the callout component :
<Box | ||
$display="inline-block" | ||
$css={css` | ||
margin-right: 0.3rem; | ||
`} | ||
> | ||
{emoji ? ( | ||
<Icon iconName={emoji} $size="16px" /> | ||
) : ( | ||
<SelectedPageIcon width={11.5} /> | ||
)} | ||
</Box> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure why adding another div inside, by adding this margin we differ from the design inside Figma.
<Box | |
$display="inline-block" | |
$css={css` | |
margin-right: 0.3rem; | |
`} | |
> | |
{emoji ? ( | |
<Icon iconName={emoji} $size="16px" /> | |
) : ( | |
<SelectedPageIcon width={11.5} /> | |
)} | |
</Box> | |
{emoji ? ( | |
<Icon iconName={emoji} $size="16px" /> | |
) : ( | |
<SelectedPageIcon width={11.5} /> | |
)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<DocIcon | ||
emojiPicker | ||
docId={doc.id} | ||
title={doc.title} | ||
emoji={emoji} | ||
$size="50px" | ||
defaultIcon={ | ||
<SimpleFileIcon | ||
width="50px" | ||
height="50px" | ||
aria-hidden="true" | ||
aria-label={t('Simple document icon')} | ||
color={colorsTokens['primary-500']} | ||
/> | ||
} | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems quite big compare to the design: https://www.figma.com/design/qdCWR4tTUr7vQSecEjCyqO/DOCS?node-id=10856-18743&t=P220L7GmYxjlkESr-0


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In our process we try to have the design provided as the single source of truth, it make it easier for design integration and then review. That say, you are welcome to add a comment in the Figma to say to Robin that you find that it would better in one way or another.
src/frontend/apps/impress/src/features/docs/doc-management/hooks/useDocTitleUpdate.tsx
Outdated
Show resolved
Hide resolved
src/frontend/apps/impress/src/features/docs/doc-header/components/DocTitle.tsx
Outdated
Show resolved
Hide resolved
src/frontend/apps/impress/src/features/docs/doc-management/hooks/useDocTitleUpdate.tsx
Show resolved
Hide resolved
src/frontend/apps/impress/src/features/docs/doc-header/components/DocTitle.tsx
Outdated
Show resolved
Hide resolved
src/frontend/apps/impress/src/features/docs/doc-management/components/DocIcon.tsx
Outdated
Show resolved
Hide resolved
src/frontend/apps/impress/src/features/docs/doc-tree/components/DocTreeItemActions.tsx
Outdated
Show resolved
Hide resolved
src/frontend/apps/impress/src/features/docs/doc-header/components/DocToolBox.tsx
Outdated
Show resolved
Hide resolved
src/frontend/apps/impress/src/features/docs/doc-management/components/DocIcon.tsx
Show resolved
Hide resolved
d52b240
to
d96fcf6
Compare
d96fcf6
to
8957c85
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments.
I will prepare a environment for Robin to review, I think it would be only next week though.
src/frontend/apps/impress/src/features/docs/doc-management/components/DocIcon.tsx
Outdated
Show resolved
Hide resolved
8957c85
to
a3f0464
Compare
a3f0464
to
f40d864
Compare
f40d864
to
06176fd
Compare
Ok @AntoLC I believe that I've taken into consideration all your comments ! |
Purpose
Adjust the behaviors of Doc title emojis in Home page, Doc tree and Doc editor following discussion on #1358 (comment)
Here is a video displaying those new behaviours
Proposal
Illustrations
External contributions
Thank you for your contribution! 🎉
Please ensure the following items are checked before submitting your pull request:
git commit --signoff
(DCO compliance)git commit -S
)<gitmoji>(type) title description
## [Unreleased]
section (if noticeable change)