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.

Sign up for GitHub

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

Jump to bottom

fix: fixed addImageBlobHook error in wwEditor (fixes: #2053) #2054

Merged
merged 5 commits into from
Jan 7, 2022
Merged

fix: fixed addImageBlobHook error in wwEditor (fixes: #2053) #2054

merged 5 commits into from
Jan 7, 2022

Conversation

Isitar
Copy link
Contributor

@Isitar Isitar commented Nov 25, 2021 •
edited
Loading

fixes #2053

Please check if the PR fulfills these requirements

  • It's the right issue type on the title
  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes/features)
  • Docs have been added/updated (for bug fixes/features)
  • It does not introduce a breaking change or has a description of the breaking change

Description

this changes the code in wwEditor to align with the code in mdEditor

@Isitar Isitar changed the title fixed addImageBlobHook error in wwEditor fix #2053: fixed addImageBlobHook error in wwEditor Nov 25, 2021
@Isitar
Copy link
Contributor Author

Isitar commented Nov 25, 2021 •
edited
Loading

The problem got introduced in 8d434ce

I understand if you don't like the if (items) but the strict comparison of the length property simply breaks this feature.
you can maybe change all if (items) to if (items?.length > 0) but if you do, please keep it consistent in all editors (md, ww, etc.)

@Isitar Isitar changed the title fix #2053: fixed addImageBlobHook error in wwEditor fix: fixed addImageBlobHook error in wwEditor (fixes: #2053) Nov 29, 2021
js87zz
js87zz approved these changes Jan 5, 2022
View reviewed changes
Copy link
Contributor

@js87zz js87zz left a comment

Choose a reason for hiding this comment

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

I checked that it was working well. :)
I'd appreciate it if you check out reviews and reflect them. And please execute "git rebase" based on master branch.
After then, I will merge this PR. Thank you! 👍

apps/editor/src/markdown/mdEditor.ts Outdated Show resolved Hide resolved
apps/editor/src/wysiwyg/wwEditor.ts Outdated Show resolved Hide resolved
@Isitar
Copy link
Contributor Author

Isitar commented Jan 5, 2022 •
edited
Loading

alright, i adjusted the code with toArray and rebased with master, ready to merge?

js87zz
js87zz approved these changes Jan 7, 2022
View reviewed changes
Copy link
Contributor

@js87zz js87zz left a comment

Choose a reason for hiding this comment

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

I checked all the modification, and I fixed the simple lint error.
When other features are developed, they will be released together.
Thank you! 👍

@js87zz js87zz merged commit 95f5e74 into nhn:master Jan 7, 2022
@Isitar Isitar deleted the bugfix/wwEdtor-blob-hook-fix branch February 10, 2022 17:24
@Isitar
Copy link
Contributor Author

Isitar commented Feb 10, 2022

you missed the fix in the relase notes ;)

@js87zz
Copy link
Contributor

js87zz commented Feb 11, 2022

@Isitar
Thanks! I added it. :)
https://github.com/nhn/tui.editor/releases/tag/editor%403.1.3

ahamelers pushed a commit to ahamelers/tui.editor that referenced this pull request Aug 21, 2023
@Isitar @js87zz
fix: fixed addImageBlobHook error in wwEditor (fixes: nhn#2053) (nhn#…
15e0391 
…2054)

* fixed addImageBlobHook error in wwEditor

* word copy paste fix

* code style: using toArray with some instead of for loop

* align code style for md and ww editor

* chore: fix broken lint

Co-authored-by: js87zz <js87zz.lee@nhn.com>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@js87zz js87zz js87zz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

addImageBlobHook only fires in Markdown-Mode
2 participants
@Isitar @js87zz