Skip to content

adding mentionTrigger #320

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

Merged
merged 5 commits into from
Jul 9, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion draft-js-mention-plugin/src/MentionSuggestions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { genKey } from 'draft-js';
import getSearchText from '../utils/getSearchText';

export default class MentionSuggestions extends Component {

static propTypes = {
entityMutability: PropTypes.oneOf([
'SEGMENTED',
Expand Down Expand Up @@ -192,6 +191,7 @@ export default class MentionSuggestions extends Component {
const newEditorState = addMention(
this.props.store.getEditorState(),
mention,
this.props.mentionTrigger,
this.props.entityMutability,
);
this.props.store.setEditorState(newEditorState);
Expand Down
11 changes: 7 additions & 4 deletions draft-js-mention-plugin/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ import defaultPositionSuggestions from './utils/positionSuggestions';

const createMentionPlugin = (config = {}) => {
const defaultTheme = {
// CSS class for mention text
mention: mentionStyles.mention,

// CSS class for suggestions component
mentionSuggestions: mentionSuggestionsStyles.mentionSuggestions,
Copy link
Member

Choose a reason for hiding this comment

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

The spaces where here by intention to make it easy to distinct which component is affected. Maybe a comment would have been better. Can you add comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't realize I made these changes. I'll add some comments.


// CSS classes for an entry in the suggestions component
mentionSuggestionsEntry: mentionSuggestionsEntryStyles.mentionSuggestionsEntry,
mentionSuggestionsEntryFocused: mentionSuggestionsEntryStyles.mentionSuggestionsEntryFocused,
mentionSuggestionsEntryText: mentionSuggestionsEntryStyles.mentionSuggestionsEntryText,
Expand Down Expand Up @@ -83,6 +84,7 @@ const createMentionPlugin = (config = {}) => {
mentionPrefix = '',
theme = defaultTheme,
positionSuggestions = defaultPositionSuggestions,
mentionTrigger = '@',
} = config;
const mentionSearchProps = {
ariaProps,
Expand All @@ -91,16 +93,17 @@ const createMentionPlugin = (config = {}) => {
store,
entityMutability: config.entityMutability ? config.entityMutability : 'SEGMENTED',
positionSuggestions,
mentionTrigger,
};
return {
MentionSuggestions: decorateComponentWithProps(MentionSuggestions, mentionSearchProps),
decorators: [
{
strategy: mentionStrategy,
strategy: mentionStrategy(mentionTrigger),
component: decorateComponentWithProps(Mention, { theme, mentionPrefix }),
},
{
strategy: mentionSuggestionsStrategy,
strategy: mentionSuggestionsStrategy(mentionTrigger),
component: decorateComponentWithProps(MentionSuggestionsPortal, { store }),
},
],
Expand Down
8 changes: 4 additions & 4 deletions draft-js-mention-plugin/src/mentionStrategy.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { Entity } from 'draft-js';

const findMention = (character) => {
const findMention = (trigger) => (character) => {
const entityKey = character.getEntity();
return (entityKey !== null && Entity.get(entityKey).getType() === 'mention');
return (entityKey !== null && Entity.get(entityKey).getType() === `mention-${trigger}`);
Copy link
Member

@nikgraf nikgraf Jun 26, 2016

Choose a reason for hiding this comment

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

This would be a breaking change to all existing implementations. I think by default the type should still be mention. I suggest we introduce another parameter to provide a custom type. I'm bit conflicted here as it probably only be used in combination with another mentionTrigger. Any ideas?

Copy link
Contributor Author

@YoonjiJang YoonjiJang Jun 26, 2016

Choose a reason for hiding this comment

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

I can't think of any use case in which a custom type is needed on its own.

Even if the mentionTrigger and type functionalities are separated, I believe it'd still be a good idea to make sure that the entity type doesn't conflict with other plugins or generic entity types, by prefixing the type string with some sort of mention plugin identifier string or by some other way? Consider https://github.com/yjang1031/draft-js-plugins/compare/master...yjang1031:mentionType

Copy link
Member

Choose a reason for hiding this comment

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

If we would change the default type everyone who used the mention type and saved it to the database would now need to re-write all save data-structures in their database. That's why the default should stick to mention.

Copy link
Member

@nikgraf nikgraf Jul 9, 2016

Choose a reason for hiding this comment

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

I will merge it and fix it, that in case of @ we keep the default of mention type. If we invest a little bit more work here, we save other people hours if not days of extra work …

};

const findMentionEntities = (contentBlock, callback) => {
contentBlock.findEntityRanges(findMention, callback);
const findMentionEntities = (trigger) => (contentBlock, callback) => {
contentBlock.findEntityRanges(findMention(trigger), callback);
};

export default findMentionEntities;
7 changes: 3 additions & 4 deletions draft-js-mention-plugin/src/mentionSuggestionsStrategy.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
/* @flow */

import findWithRegex from 'find-with-regex';
import _ from 'lodash';
Copy link
Member

@nikgraf nikgraf Jul 9, 2016

Choose a reason for hiding this comment

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

This is not ideal, as it would include the whole lodash library. By using import escapeRegExp from 'lodash/escapeRegExp' we can reduce the imported code and skip all other lodash code not necessary for this plugin.


const MENTION_REGEX = /(\s|^)@[\w]*/g;

export default (contentBlock: Object, callback: Function) => {
findWithRegex(MENTION_REGEX, contentBlock, callback);
export default (trigger: String) => (contentBlock: Object, callback: Function) => {
findWithRegex(new RegExp(`(\\s|^)${_.escapeRegExp(trigger)}[\\w]*`, 'g'), contentBlock, callback);
};
4 changes: 2 additions & 2 deletions draft-js-mention-plugin/src/modifiers/addMention.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Modifier, EditorState, Entity } from 'draft-js';
import getSearchText from '../utils/getSearchText';

const addMention = (editorState, mention, entityMutability) => {
const entityKey = Entity.create('mention', entityMutability, { mention });
const addMention = (editorState, mention, mentionTrigger, entityMutability) => {
const entityKey = Entity.create(`mention-${mentionTrigger}`, entityMutability, { mention });

const currentSelectionState = editorState.getSelection();
const { begin, end } = getSearchText(editorState, currentSelectionState);
Expand Down