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

feat(rich-text-editor): DLT-2116 enable emoji searching by name and keyword #1

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

Conversation

lizard-boy
Copy link

Enable emoji searching by name and keyword

Obligatory GIF (super important!)

Obligatory GIF

🛠️ Type Of Change

These types will increment the version number on release:

  • Feature

📖 Jira Ticket

https://dialpad.atlassian.net/browse/DLT-2116

📖 Description

  • Added the ability to search emojis through suggestion with keywords and name too.
  • Added suggestionLimit to limit the results to 20, this hugely improves the performance.

💡 Context

  • Issues with message input were reported

📝 Checklist

For all PRs:

  • I have ensured no private Dialpad links or info are in the code or pull request description (Dialtone is a public repo!).
  • I have reviewed my changes.
  • I have added all relevant documentation.
  • I have considered the performance impact of my change.

For all Vue changes:

  • I have added / updated unit tests.
  • I have made my changes in Vue 2 and Vue 3. Note: you may sync your changes from Vue 2 to Vue 3 (or vice versa) using the ./scripts/dialtone-vue-sync.sh script. Read docs here: Dialtone Vue Sync Script
  • I have validated components with a screen reader.
  • I have validated components keyboard navigation.

🔮 Next Steps

Release, update and test on product

📷 Screenshots / GIFs

Screenshot 2024-10-04 at 2 25 04 p m

Signed-off-by: Julio Ortega <[email protected]>
Signed-off-by: Julio Ortega <[email protected]>
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Enhanced emoji suggestion functionality in the rich text editor for both Vue 2 and Vue 3 versions.

  • Added keyword and name-based emoji searching in packages/dialtone-vue2/components/rich_text_editor/extensions/emoji/suggestion.js and Vue 3 equivalent
  • Implemented a 20-result suggestion limit to improve performance
  • Updated filtering logic to accommodate new search capabilities
  • Changes are identical in both Vue 2 and Vue 3 files

2 file(s) reviewed, 6 comment(s)
Edit PR Review Bot Settings

return false;
});
return filteredEmoji.map(item => { return { code: item.shortname }; });
query = query.toLowerCase();
Copy link

Choose a reason for hiding this comment

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

style: Reassigning function parameter. Consider using a new variable name

Comment on lines +20 to +27
const filteredEmoji = emojiList
.filter(
item => [
item.name,
item.shortname.replaceAll(':', ''),
...item.keywords,
].some(text => text.startsWith(query)),
).splice(0, suggestionLimit);
Copy link

Choose a reason for hiding this comment

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

style: This filtering could be expensive for large emoji lists. Consider memoization or indexing for better performance

.filter(
item => [
item.name,
item.shortname.replaceAll(':', ''),
Copy link

Choose a reason for hiding this comment

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

style: replaceAll might not be supported in all environments. Consider using replace with regex

return false;
});
return filteredEmoji.map(item => { return { code: item.shortname }; });
query = query.toLowerCase();
Copy link

Choose a reason for hiding this comment

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

style: Reassigning function parameter. Consider using a new variable name

Comment on lines +21 to +28
const filteredEmoji = emojiList
.filter(
item => [
item.name,
item.shortname.replaceAll(':', ''),
...item.keywords,
].some(text => text.startsWith(query)),
).splice(0, suggestionLimit);
Copy link

Choose a reason for hiding this comment

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

style: This filtering logic might be computationally expensive for large emoji lists. Consider optimizing if performance issues arise

.filter(
item => [
item.name,
item.shortname.replaceAll(':', ''),
Copy link

Choose a reason for hiding this comment

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

style: replaceAll may not be supported in all browsers. Consider using replace with a regex for better compatibility

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.

2 participants