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

MARKET-1671 Emoji component #771

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

MARKET-1671 Emoji component #771

wants to merge 3 commits into from

Conversation

Saharij
Copy link
Collaborator

@Saharij Saharij commented Sep 18, 2023

Comment on lines 44 to 52
{ pickerVisibility ? (
<svg width={ validButtonSize } height={ validButtonSize } viewBox="0 0 512 512" fill={ buttonColor }>
<path d="M464 256A208 208 0 1 0 48 256a208 208 0 1 0 416 0zM0 256a256 256 0 1 1 512 0A256 256 0 1 1 0 256zm349.5 52.4c18.7-4.4 35.9 12 25.5 28.1C350.4 374.6 306.3 400 255.9 400s-94.5-25.4-119.1-63.5c-10.4-16.1 6.8-32.5 25.5-28.1c28.9 6.8 60.5 10.5 93.6 10.5s64.7-3.7 93.6-10.5zM144.4 208a32 32 0 1 1 64 0 32 32 0 1 1 -64 0zm165.8 21.7c-7.6 8.1-20.2 8.5-28.3 .9s-8.5-20.2-.9-28.3c14.5-15.5 35.2-22.3 54.6-22.3s40.1 6.8 54.6 22.3c7.6 8.1 7.1 20.7-.9 28.3s-20.7 7.1-28.3-.9c-5.5-5.8-14.8-9.7-25.4-9.7s-19.9 3.8-25.4 9.7z" />
</svg>
) : (
<svg width={ validButtonSize } height={ validButtonSize } viewBox="0 0 512 512" fill={ buttonColor }>
<path d="M464 256A208 208 0 1 0 48 256a208 208 0 1 0 416 0zM0 256a256 256 0 1 1 512 0A256 256 0 1 1 0 256zm177.6 62.1C192.8 334.5 218.8 352 256 352s63.2-17.5 78.4-33.9c9-9.7 24.2-10.4 33.9-1.4s10.4 24.2 1.4 33.9c-22 23.8-60 49.4-113.6 49.4s-91.7-25.5-113.6-49.4c-9-9.7-8.4-24.9 1.4-33.9s24.9-8.4 33.9 1.4zM144.4 208a32 32 0 1 1 64 0 32 32 0 1 1 -64 0zm192-32a32 32 0 1 1 0 64 32 32 0 1 1 0-64z"/>
</svg>
) }
Copy link
Contributor

Choose a reason for hiding this comment

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

It will be better to make this more readable.

You can make for SVG separable components. Also, you can move the condition from return

Comment on lines 27 to 28
transform: scale(1.2);
transition: transform .3s;
Copy link
Contributor

Choose a reason for hiding this comment

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

can be variables


&__dropdown {
position: absolute;
z-index: 100;
Copy link
Contributor

Choose a reason for hiding this comment

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

can be variable

width: auto;

&--disabled {
opacity: 0.38;
Copy link
Contributor

Choose a reason for hiding this comment

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

can be variable

| Theme<br/>`theme` | Select [Light:`light`<br/>Dark:`dark`<br/>Auto:`auto`] | Light:`light` | Theme Logic | YES | YES | This handler allows you to select a theme for the emoji dropdown list. |
| Emoji Style<br/>`emojiStyle` | Select [Apple:`apple`<br/>Google:`google`<br/>Facebook:`facebook`] | Apple:`apple` | Emoji Style Logic | YES | YES | This handler allows you to select an emoji style. |
| Search Disabled<br/>`searchDisabled` | Checkbox | `true` | Search Disabled Logic | YES | YES | This handler allows you to disable the emoji search field. |

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it easy and obvious to implement binding emojis with default input? If no - need an instruction how to do it.


| Name | Triggers | Context Blocks |
|-----------------------|---------------------------|----------------------------------------------------|
| On Emoji Select Event | when an emoji is selected | Emoji: `String`, Emoji Names: [`String`, `String`] |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you provide example values that returns from context blocks?

Comment on lines +43 to +47
<div onClick={ handleEmojiButtonClick } className="emoji-picker__button">
<svg width={ validButtonSize } height={ validButtonSize } viewBox="0 0 512 512" fill={ buttonColor }>
<path d={ getIconPath(pickerVisibility) } />
</svg>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Component?

Comment on lines +48 to +59
{ pickerVisibility &&
<div className={ cn("emoji-picker__dropdown", `emoji-picker__dropdown-position--${ dropdownPosition }`) }>
<EmojiPicker
theme={ theme }
emojiStyle={ emojiStyle }
width={ validDropdownWidth }
height={ validDropdownHeight }
searchDisabled={ searchDisabled }
onEmojiClick={ handleEmojiClick }
/>
</div>
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
{ pickerVisibility &&
<div className={ cn("emoji-picker__dropdown", `emoji-picker__dropdown-position--${ dropdownPosition }`) }>
<EmojiPicker
theme={ theme }
emojiStyle={ emojiStyle }
width={ validDropdownWidth }
height={ validDropdownHeight }
searchDisabled={ searchDisabled }
onEmojiClick={ handleEmojiClick }
/>
</div>
}
<PickerLayoutManager dropdownPosition={dropdownPosition} pickerVisibility={pickerVisibility}>
<EmojiPicker
theme={ theme }
emojiStyle={ emojiStyle }
width={ validDropdownWidth }
height={ validDropdownHeight }
searchDisabled={ searchDisabled }
onEmojiClick={ handleEmojiClick }
/>
</PickerLayoutManager>

Comment on lines +31 to +33
if (!display) {
return null;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

For now it's impossible to always show it? It could be shown only after some trigger?
If yes need to add this possibility

const validDropdownWidth = useMemo(() => normalizeDimensionValue(dropdownWidth), [dropdownWidth]);
const validDropdownHeight = useMemo(() => normalizeDimensionValue(dropdownHeight), [dropdownHeight]);

const handleEmojiButtonClick = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

togglePickerVisibility

setPickerVisibility(prevState => !prevState);
};

const handleEmojiClick = event => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

onEmojiClick

};

const handleEmojiClick = event => {
const { emoji, names } = event;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could be destructed at arguments

"type": "custom",
"category": "Custom Components",
"properties": [
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need actions and property that could control picker visibility

const handleEmojiClick = event => {
const { emoji, names } = event;

navigator.clipboard.writeText(emoji);
Copy link
Collaborator

Choose a reason for hiding this comment

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

But how it will be inserted into text fields? Pasting? It may not be convenient.
We should try to find a way to link it with input and write an emoji directly into it, without a clipboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants