-
Notifications
You must be signed in to change notification settings - Fork 3
feat: add a few function to utils for count #21
Conversation
|
juliankrispel
left a comment
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.
Looks great! I'll help with getting it merged 👍 - if you could address some of the concerns that'd be fabbb!
examples/counter/src/index.js
Outdated
| onChange={this.onChange}> | ||
| <div>Char count: {getCharCount(this.state.editorState)}</div> | ||
| <div>Word count: {getWordCount(this.state.editorState)}</div> | ||
| <div>Line count: {getLineCount(this.state.editorState)}</div> |
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.
Make this a separate component WordCountPlugin.js
export default () => <Plugin>{({ editorState }) => <Fragment>
<div>Char count: {getCharCount(this.state.editorState)}</div>
<div>Word count: {getWordCount(this.state.editorState)}</div>
<div>Line count: {getLineCount(this.state.editorState)}</div>
</Fragment>
And import it to index.js
packages/utils/dist/index.es.js
Outdated
| @@ -1,4 +1,5 @@ | |||
| import { AtomicBlockUtils, RichUtils, EditorState, Modifier } from 'draft-js'; | |||
| import punycode from 'punycode'; | |||
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.
How big is this dependency? Also, it needs to be added to external in the rollup config
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.
why do we need this exactly?
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.
This dependency used for handle unicode characters.
It's size is 142B(MINIFIED). Still need to added it into external?
packages/utils/dist/index.js
Outdated
| @@ -1,8 +1,11 @@ | |||
| 'use strict'; | |||
|
|
|||
| Object.defineProperty(exports, '__esModule', { value: true }); | |||
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.
dist folder should be ignored (let's check the .gitignore file
packages/utils/package.json
Outdated
| { | ||
| "name": "@djsp/utils", | ||
| "version": "0.0.3", | ||
| "version": "0.0.4", |
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.
this will update after a rebase
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.
Ah, after git rebase master, the version updated to 0.1.5. Not sure if it is right
packages/utils/src/index.js
Outdated
| Modifier, | ||
| } from 'draft-js' | ||
| import type DraftEntityInstance from 'draft-js/lib/DraftEntityInstance' | ||
| // eslint-disable-next-line |
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.
shouldn't need to use the eslint comment here
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.
Eslint will throw error about deprecated-api. So i add eslint-disable-next-line node/no-deprecated-api in the next commit
| return entity && entity.getType() === entityType | ||
| } | ||
|
|
||
| export function getCharCount(editorState: EditorState): number { |
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.
does this pass flow?
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.
Currently, @djsp/utils can not pass flow. Looks like missing some files in flow-lib
41e401a to
db9b69e
Compare
db9b69e to
19ec241
Compare
Resolve #23