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

fix(factories): Allow alphanumeric IDs for suggestion nodes in createSuggestionExtension #66

Merged
merged 3 commits into from
Dec 19, 2022

Conversation

rfgamaral
Copy link
Member

@rfgamaral rfgamaral commented Dec 15, 2022

Overview

Fixes #64.

PR Checklist

  • Changes introduced here have been manually tested by someone other than the PR author
  • Pull request title follows the Conventional Commits Specification
  • Added/updated unit test cases and/or end-to-end test cases
  • Added/updated documentation to Storybook or README.md

Test plan

  • Open the preview Storybook deployed to Netlify
  • Open the Rich-text → Default story
  • Add [John Doe](mention://@user123:matrix.org) in the content control (bottom)
    • Observe that the editor was initialized with a mention suggestion identified
    • Observe that the Markdown output shows [John Doe](mention://@user123:matrix.org)

@rfgamaral rfgamaral added the 🙋 Ask PR Used for PRs that need a review before merging. label Dec 15, 2022
@rfgamaral rfgamaral self-assigned this Dec 15, 2022
@netlify
Copy link

netlify bot commented Dec 15, 2022

Deploy Preview for doist-typist ready!

Name Link
🔨 Latest commit 8b68b38
🔍 Latest deploy log https://app.netlify.com/sites/doist-typist/deploys/639caa2a4414e40008d8b028
😎 Deploy Preview https://deploy-preview-66--doist-typist.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@rfgamaral rfgamaral requested review from a team and frankieyan and removed request for a team December 15, 2022 18:10
@rfgamaral rfgamaral marked this pull request as ready for review December 15, 2022 18:10
Copy link
Member

@frankieyan frankieyan left a comment

Choose a reason for hiding this comment

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

Works well 👍

I suppose this would be a minor rather than breaking change as this doesn't change the markdown serialization, and we can continue to use numeric ids as they would be converted to strings internally by the extension, correct?

@rfgamaral
Copy link
Member Author

rfgamaral commented Dec 16, 2022

I suppose this would be a minor rather than breaking change as this doesn't change the markdown serialization, and we can continue to use numeric ids as they would be converted to strings internally by the extension, correct?

Yes, when storage.itemsById is populated, keys are converted to strings.


But I'm glad you asked this because it prompted me to review the attributesMapping constraints in createSuggestionExtension, and I've realized that I broke the mapping for number properties in this PR. To explain what I mean, imagine SuggestionItem is defined like this:

type SuggestionItem = {
    uid: number
    name: string
}

But, internally, the extension uses id, so we need to map one property to the other, like this:

createSuggestionExtension<SuggestionItem>(MENTION_SUGGESTION_TYPE, MENTION_SUGGESTION_ITEMS, {
    id: 'uid',
    label: 'name',
})

However, with the changes I made in this PR, this doesn't work:

image

Because we are trying to map a number (i.e. uid), to a string (i.e. both id and label only allow strings).

So, technically, this is a breaking change, but it wouldn't actually break anything for us because our apps use id by default, and we don't need to map this attribute in particular, and since the extension will take care of converting everything to strings, everything just keeps working.

However, there's no reason to introduce this breaking change, even if it doesn't affect us. It's a simple fix, and doesn't break anything for anyone. I've also updated the MentionSuggestion and HashtagSuggestion extensions to serve as examples for different implementations.

Just thought I'd leave this whole clarification here for posterity.


@frankieyan Can you please take a second look at this commit?

@rfgamaral
Copy link
Member Author

@frankieyan Made another commit, shouldn't change anything, but please double-check 🙏

@frankieyan
Copy link
Member

frankieyan commented Dec 16, 2022

Thanks for the explanation 👍 both hashtags and mentions are still working in storybook 🚀

@rfgamaral rfgamaral merged commit a1726a6 into main Dec 19, 2022
@rfgamaral rfgamaral deleted the ricardo/issue-64 branch December 19, 2022 12:33
doistbot added a commit that referenced this pull request Dec 19, 2022
## [1.0.8](v1.0.7...v1.0.8) (2022-12-19)

### Bug Fixes

* **factories:** Allow alphanumeric IDs for suggestion nodes in `createSuggestionExtension` ([#66](#66)) ([a1726a6](a1726a6))
@doistbot
Copy link
Member

🎉 This PR is included in version 1.0.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

@doistbot doistbot added the released Pull requests that have been released to production label Dec 19, 2022
jerrywcy added a commit to jerrywcy/memos-typist-editor that referenced this pull request Aug 12, 2023
## 1.0.0 (2023-08-12)

### Features

* Add the `PasteHTMLTableAsString` extension ([Doist#290](https://github.com/jerrywcy/memos-typist-editor/issues/290)) ([ee90014](ee90014))
* Allow all marks to coexist with the Code mark ([Doist#309](https://github.com/jerrywcy/memos-typist-editor/issues/309)) ([ac06735](ac06735))
* Disallow suggestions inside inline code marks and code blocks ([Doist#154](https://github.com/jerrywcy/memos-typist-editor/issues/154)) ([7d75314](7d75314))
* Initial Typist implementation ([19451ab](19451ab))
* **serializers:** Add `get*` functions for reusable singular instances ([Doist#88](https://github.com/jerrywcy/memos-typist-editor/issues/88)) ([b2c77c3](b2c77c3))
* TaskItem and TaskList extension ([5db1900](5db1900))

### Bug Fixes

* `insertMarkdownContent` didn't insert Markdown correctly in plain-text documents ([Doist#13](https://github.com/jerrywcy/memos-typist-editor/issues/13)) ([74cc623](74cc623))
* Add required ProseMirror dependencies to package ([Doist#73](https://github.com/jerrywcy/memos-typist-editor/issues/73)) ([cd605c0](cd605c0))
* Add support for literal autolinks (GFM based) ([Doist#303](https://github.com/jerrywcy/memos-typist-editor/issues/303)) ([4537091](4537091))
* Add support for query params in `RichTextLink` extension ([Doist#4](https://github.com/jerrywcy/memos-typist-editor/issues/4)) ([9fac158](9fac158))
* Bold and Italic now use *** and ___ instead of ** and __ ([cc29ea9](cc29ea9))
* change to memos-typist-editor in package.json ([c8250b1](c8250b1))
* **deps:** Migrate ProseMirror dependencies to `@tiptap/pm` package ([Doist#151](https://github.com/jerrywcy/memos-typist-editor/issues/151)) ([d2a8eae](d2a8eae))
* **deps:** update dependency prosemirror-codemark to v0.4.2 ([Doist#34](https://github.com/jerrywcy/memos-typist-editor/issues/34)) ([58938a1](58938a1))
* **deps:** update dependency prosemirror-model to v1.18.2 ([Doist#25](https://github.com/jerrywcy/memos-typist-editor/issues/25)) ([5d1fc1b](5d1fc1b))
* **deps:** update dependency prosemirror-model to v1.18.3 ([Doist#30](https://github.com/jerrywcy/memos-typist-editor/issues/30)) ([54bfd56](54bfd56))
* **deps:** update dependency prosemirror-view to v1.29.1 ([Doist#26](https://github.com/jerrywcy/memos-typist-editor/issues/26)) ([9f86a5e](9f86a5e))
* **deps:** update dependency unist-util-remove to v4 ([Doist#380](https://github.com/jerrywcy/memos-typist-editor/issues/380)) ([6c5430a](6c5430a))
* **deps:** update gfm autolink literal packages to v2 (major) ([Doist#378](https://github.com/jerrywcy/memos-typist-editor/issues/378)) ([7ad9af8](7ad9af8))
* **deps:** update gfm strikethrough packages to v2 (major) ([Doist#379](https://github.com/jerrywcy/memos-typist-editor/issues/379)) ([b1e411d](b1e411d))
* **deps:** update tiptap packages to v2.0.0 ([Doist#198](https://github.com/jerrywcy/memos-typist-editor/issues/198)) ([fe4aa82](fe4aa82))
* **deps:** update tiptap packages to v2.0.0-beta.200 ([Doist#3](https://github.com/jerrywcy/memos-typist-editor/issues/3)) ([6e977a9](6e977a9))
* **deps:** update tiptap packages to v2.0.0-beta.202 ([Doist#9](https://github.com/jerrywcy/memos-typist-editor/issues/9)) ([ce43f74](ce43f74))
* **deps:** update tiptap packages to v2.0.0-beta.203 ([Doist#35](https://github.com/jerrywcy/memos-typist-editor/issues/35)) ([2188bc6](2188bc6))
* **deps:** update tiptap packages to v2.0.0-beta.204 ([Doist#38](https://github.com/jerrywcy/memos-typist-editor/issues/38)) ([cb5b359](cb5b359))
* **deps:** update tiptap packages to v2.0.0-beta.205 ([Doist#54](https://github.com/jerrywcy/memos-typist-editor/issues/54)) ([2074402](2074402))
* **deps:** update tiptap packages to v2.0.0-beta.206 ([Doist#59](https://github.com/jerrywcy/memos-typist-editor/issues/59)) ([27e0f26](27e0f26))
* **deps:** update tiptap packages to v2.0.0-beta.207 ([Doist#63](https://github.com/jerrywcy/memos-typist-editor/issues/63)) ([da9889f](da9889f))
* **deps:** update tiptap packages to v2.0.0-beta.209 ([Doist#75](https://github.com/jerrywcy/memos-typist-editor/issues/75)) ([97a41c5](97a41c5))
* **deps:** update tiptap packages to v2.0.0-beta.215 ([Doist#129](https://github.com/jerrywcy/memos-typist-editor/issues/129)) ([af22d22](af22d22))
* **deps:** update tiptap packages to v2.0.0-beta.216 ([Doist#135](https://github.com/jerrywcy/memos-typist-editor/issues/135)) ([3d31a2e](3d31a2e))
* **deps:** update tiptap packages to v2.0.0-beta.217 ([Doist#136](https://github.com/jerrywcy/memos-typist-editor/issues/136)) ([eebba15](eebba15))
* **deps:** update tiptap packages to v2.0.0-beta.218 ([Doist#148](https://github.com/jerrywcy/memos-typist-editor/issues/148)) ([0e9e179](0e9e179))
* **deps:** update tiptap packages to v2.0.0-beta.220 ([Doist#158](https://github.com/jerrywcy/memos-typist-editor/issues/158)) ([c7fc3d8](c7fc3d8))
* **deps:** update tiptap packages to v2.0.0-rc.1 ([Doist#192](https://github.com/jerrywcy/memos-typist-editor/issues/192)) ([26d090a](26d090a))
* **deps:** update tiptap packages to v2.0.0-rc.2 ([Doist#195](https://github.com/jerrywcy/memos-typist-editor/issues/195)) ([8753f13](8753f13))
* **deps:** update tiptap packages to v2.0.1 ([Doist#201](https://github.com/jerrywcy/memos-typist-editor/issues/201)) ([e31cb2f](e31cb2f))
* **deps:** update tiptap packages to v2.0.2 ([Doist#212](https://github.com/jerrywcy/memos-typist-editor/issues/212)) ([200b339](200b339))
* **deps:** update tiptap packages to v2.0.3 ([Doist#226](https://github.com/jerrywcy/memos-typist-editor/issues/226)) ([a1953b0](a1953b0))
* **docs:** correct the render function name ([Doist#50](https://github.com/jerrywcy/memos-typist-editor/issues/50)) ([45dd681](45dd681))
* Extra paragraph node inserted above an Horizontal Rule ([Doist#313](https://github.com/jerrywcy/memos-typist-editor/issues/313)) ([3852309](3852309))
* **factories:** Allow alphanumeric IDs for suggestion nodes in `createSuggestionExtension` ([Doist#66](https://github.com/jerrywcy/memos-typist-editor/issues/66)) ([a1726a6](a1726a6))
* **html-serializer:** Disables tokenizers if marks/nodes are not found in the editor schema ([Doist#86](https://github.com/jerrywcy/memos-typist-editor/issues/86)) ([0ed4a9b](0ed4a9b))
* **html-serializer:** Don't share instances between editors ([Doist#275](https://github.com/jerrywcy/memos-typist-editor/issues/275)) ([3aba8c7](3aba8c7))
* Invalid `hasCodeMarkBefore` check in `canInsertSuggestion` utility function ([Doist#156](https://github.com/jerrywcy/memos-typist-editor/issues/156)) ([21826c5](21826c5))
* **markdown-serializer:** Override Turndown escaping behaviour with custom rules ([Doist#102](https://github.com/jerrywcy/memos-typist-editor/issues/102)) ([6950afb](6950afb))
* **paste-markdown:** Incorrect paste behaviour when HTML source is VSCode ([Doist#260](https://github.com/jerrywcy/memos-typist-editor/issues/260)) ([3326796](3326796))
* Remove unused Tippy.js peer dependency ([Doist#56](https://github.com/jerrywcy/memos-typist-editor/issues/56)) ([85f87a5](85f87a5))
* Replace usage of useEvent with useCallback ([Doist#98](https://github.com/jerrywcy/memos-typist-editor/issues/98)) ([3b175f7](3b175f7))
* **rich-text-link:** More lenient regex for input/paste rule ([Doist#72](https://github.com/jerrywcy/memos-typist-editor/issues/72)) ([98e363f](98e363f))
* Task List now only toggle by typing `[ ]` in bullet list ([ae3103b](ae3103b))
* Unit-tests now support TaskItem and TaskList ([639abf6](639abf6))

### Reverts

* Revert "ci: Add support to publish experimental releases" (Doist#279) ([dc57964](dc57964)), closes [Doist#279](https://github.com/jerrywcy/memos-typist-editor/issues/279) [Doist#278](https://github.com/jerrywcy/memos-typist-editor/issues/278)
jerrywcy added a commit to jerrywcy/memos-typist-editor that referenced this pull request Aug 12, 2023
## 1.0.0 (2023-08-12)

### Features

* Add the `PasteHTMLTableAsString` extension ([Doist#290](https://github.com/jerrywcy/memos-typist-editor/issues/290)) ([ee90014](ee90014))
* Allow all marks to coexist with the Code mark ([Doist#309](https://github.com/jerrywcy/memos-typist-editor/issues/309)) ([ac06735](ac06735))
* Disallow suggestions inside inline code marks and code blocks ([Doist#154](https://github.com/jerrywcy/memos-typist-editor/issues/154)) ([7d75314](7d75314))
* Initial Typist implementation ([19451ab](19451ab))
* **serializers:** Add `get*` functions for reusable singular instances ([Doist#88](https://github.com/jerrywcy/memos-typist-editor/issues/88)) ([b2c77c3](b2c77c3))
* TaskItem and TaskList extension ([5db1900](5db1900))

### Bug Fixes

* `insertMarkdownContent` didn't insert Markdown correctly in plain-text documents ([Doist#13](https://github.com/jerrywcy/memos-typist-editor/issues/13)) ([74cc623](74cc623))
* Add required ProseMirror dependencies to package ([Doist#73](https://github.com/jerrywcy/memos-typist-editor/issues/73)) ([cd605c0](cd605c0))
* Add support for literal autolinks (GFM based) ([Doist#303](https://github.com/jerrywcy/memos-typist-editor/issues/303)) ([4537091](4537091))
* Add support for query params in `RichTextLink` extension ([Doist#4](https://github.com/jerrywcy/memos-typist-editor/issues/4)) ([9fac158](9fac158))
* Bold and Italic now use *** and ___ instead of ** and __ ([cc29ea9](cc29ea9))
* change description in package.json ([52a2535](52a2535))
* change to memos-typist-editor in package.json ([c8250b1](c8250b1))
* **deps:** Migrate ProseMirror dependencies to `@tiptap/pm` package ([Doist#151](https://github.com/jerrywcy/memos-typist-editor/issues/151)) ([d2a8eae](d2a8eae))
* **deps:** update dependency prosemirror-codemark to v0.4.2 ([Doist#34](https://github.com/jerrywcy/memos-typist-editor/issues/34)) ([58938a1](58938a1))
* **deps:** update dependency prosemirror-model to v1.18.2 ([Doist#25](https://github.com/jerrywcy/memos-typist-editor/issues/25)) ([5d1fc1b](5d1fc1b))
* **deps:** update dependency prosemirror-model to v1.18.3 ([Doist#30](https://github.com/jerrywcy/memos-typist-editor/issues/30)) ([54bfd56](54bfd56))
* **deps:** update dependency prosemirror-view to v1.29.1 ([Doist#26](https://github.com/jerrywcy/memos-typist-editor/issues/26)) ([9f86a5e](9f86a5e))
* **deps:** update dependency unist-util-remove to v4 ([Doist#380](https://github.com/jerrywcy/memos-typist-editor/issues/380)) ([6c5430a](6c5430a))
* **deps:** update gfm autolink literal packages to v2 (major) ([Doist#378](https://github.com/jerrywcy/memos-typist-editor/issues/378)) ([7ad9af8](7ad9af8))
* **deps:** update gfm strikethrough packages to v2 (major) ([Doist#379](https://github.com/jerrywcy/memos-typist-editor/issues/379)) ([b1e411d](b1e411d))
* **deps:** update tiptap packages to v2.0.0 ([Doist#198](https://github.com/jerrywcy/memos-typist-editor/issues/198)) ([fe4aa82](fe4aa82))
* **deps:** update tiptap packages to v2.0.0-beta.200 ([Doist#3](https://github.com/jerrywcy/memos-typist-editor/issues/3)) ([6e977a9](6e977a9))
* **deps:** update tiptap packages to v2.0.0-beta.202 ([Doist#9](https://github.com/jerrywcy/memos-typist-editor/issues/9)) ([ce43f74](ce43f74))
* **deps:** update tiptap packages to v2.0.0-beta.203 ([Doist#35](https://github.com/jerrywcy/memos-typist-editor/issues/35)) ([2188bc6](2188bc6))
* **deps:** update tiptap packages to v2.0.0-beta.204 ([Doist#38](https://github.com/jerrywcy/memos-typist-editor/issues/38)) ([cb5b359](cb5b359))
* **deps:** update tiptap packages to v2.0.0-beta.205 ([Doist#54](https://github.com/jerrywcy/memos-typist-editor/issues/54)) ([2074402](2074402))
* **deps:** update tiptap packages to v2.0.0-beta.206 ([Doist#59](https://github.com/jerrywcy/memos-typist-editor/issues/59)) ([27e0f26](27e0f26))
* **deps:** update tiptap packages to v2.0.0-beta.207 ([Doist#63](https://github.com/jerrywcy/memos-typist-editor/issues/63)) ([da9889f](da9889f))
* **deps:** update tiptap packages to v2.0.0-beta.209 ([Doist#75](https://github.com/jerrywcy/memos-typist-editor/issues/75)) ([97a41c5](97a41c5))
* **deps:** update tiptap packages to v2.0.0-beta.215 ([Doist#129](https://github.com/jerrywcy/memos-typist-editor/issues/129)) ([af22d22](af22d22))
* **deps:** update tiptap packages to v2.0.0-beta.216 ([Doist#135](https://github.com/jerrywcy/memos-typist-editor/issues/135)) ([3d31a2e](3d31a2e))
* **deps:** update tiptap packages to v2.0.0-beta.217 ([Doist#136](https://github.com/jerrywcy/memos-typist-editor/issues/136)) ([eebba15](eebba15))
* **deps:** update tiptap packages to v2.0.0-beta.218 ([Doist#148](https://github.com/jerrywcy/memos-typist-editor/issues/148)) ([0e9e179](0e9e179))
* **deps:** update tiptap packages to v2.0.0-beta.220 ([Doist#158](https://github.com/jerrywcy/memos-typist-editor/issues/158)) ([c7fc3d8](c7fc3d8))
* **deps:** update tiptap packages to v2.0.0-rc.1 ([Doist#192](https://github.com/jerrywcy/memos-typist-editor/issues/192)) ([26d090a](26d090a))
* **deps:** update tiptap packages to v2.0.0-rc.2 ([Doist#195](https://github.com/jerrywcy/memos-typist-editor/issues/195)) ([8753f13](8753f13))
* **deps:** update tiptap packages to v2.0.1 ([Doist#201](https://github.com/jerrywcy/memos-typist-editor/issues/201)) ([e31cb2f](e31cb2f))
* **deps:** update tiptap packages to v2.0.2 ([Doist#212](https://github.com/jerrywcy/memos-typist-editor/issues/212)) ([200b339](200b339))
* **deps:** update tiptap packages to v2.0.3 ([Doist#226](https://github.com/jerrywcy/memos-typist-editor/issues/226)) ([a1953b0](a1953b0))
* **docs:** correct the render function name ([Doist#50](https://github.com/jerrywcy/memos-typist-editor/issues/50)) ([45dd681](45dd681))
* Extra paragraph node inserted above an Horizontal Rule ([Doist#313](https://github.com/jerrywcy/memos-typist-editor/issues/313)) ([3852309](3852309))
* **factories:** Allow alphanumeric IDs for suggestion nodes in `createSuggestionExtension` ([Doist#66](https://github.com/jerrywcy/memos-typist-editor/issues/66)) ([a1726a6](a1726a6))
* **html-serializer:** Disables tokenizers if marks/nodes are not found in the editor schema ([Doist#86](https://github.com/jerrywcy/memos-typist-editor/issues/86)) ([0ed4a9b](0ed4a9b))
* **html-serializer:** Don't share instances between editors ([Doist#275](https://github.com/jerrywcy/memos-typist-editor/issues/275)) ([3aba8c7](3aba8c7))
* Invalid `hasCodeMarkBefore` check in `canInsertSuggestion` utility function ([Doist#156](https://github.com/jerrywcy/memos-typist-editor/issues/156)) ([21826c5](21826c5))
* **markdown-serializer:** Override Turndown escaping behaviour with custom rules ([Doist#102](https://github.com/jerrywcy/memos-typist-editor/issues/102)) ([6950afb](6950afb))
* **paste-markdown:** Incorrect paste behaviour when HTML source is VSCode ([Doist#260](https://github.com/jerrywcy/memos-typist-editor/issues/260)) ([3326796](3326796))
* Remove unused Tippy.js peer dependency ([Doist#56](https://github.com/jerrywcy/memos-typist-editor/issues/56)) ([85f87a5](85f87a5))
* Replace usage of useEvent with useCallback ([Doist#98](https://github.com/jerrywcy/memos-typist-editor/issues/98)) ([3b175f7](3b175f7))
* **rich-text-link:** More lenient regex for input/paste rule ([Doist#72](https://github.com/jerrywcy/memos-typist-editor/issues/72)) ([98e363f](98e363f))
* Task List now only toggle by typing `[ ]` in bullet list ([ae3103b](ae3103b))
* Unit-tests now support TaskItem and TaskList ([639abf6](639abf6))

### Reverts

* Revert "ci: Add support to publish experimental releases" (Doist#279) ([dc57964](dc57964)), closes [Doist#279](https://github.com/jerrywcy/memos-typist-editor/issues/279) [Doist#278](https://github.com/jerrywcy/memos-typist-editor/issues/278)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🙋 Ask PR Used for PRs that need a review before merging. released Pull requests that have been released to production
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to modify the selected suggestion node
3 participants