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: #3540 Ability to preserve marks on lists #3541

Merged
merged 6 commits into from
Feb 22, 2023

Conversation

gethari
Copy link
Contributor

@gethari gethari commented Dec 14, 2022

Fixes #3540

Attaching a sample clip on the demo.

  • When we have a paragraph with marks, and when the user moves to next line and toggle any list item, the marks can be preserved based on the keepMarks flag similar to hardBreak extension
chrome_PJzxiKCJzg.mp4

@netlify
Copy link

netlify bot commented Dec 14, 2022

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit 50b95b9
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/63ea4a74b8ae0a0008f124df
😎 Deploy Preview https://deploy-preview-3541--tiptap-embed.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.

@gethari
Copy link
Contributor Author

gethari commented Dec 14, 2022

@bdbch can you please review this when you get a chance, also do let me know if it would be ideal to do the same keepMarks for TaskList as well 🚀

}
if (config.keepAttributes) {
/** If the nodeType is `bulletList` or `orderedList` set the `nodeType` as `listItem` */
const nodeType = config.type.name === 'bulletList' || config.type.name === 'orderedList' ? 'listItem' : 'taskList'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bdbch I'm just worried about this piece of code, please let me know if there's any better way of doing this. The idea here is

  • When there's a paragraph with say colored text
  • When we switch over to 2nd line, we apply the color to the whole <li> only then it is color is also applied for the list-style indicator.

@bdbch
Copy link
Contributor

bdbch commented Dec 16, 2022

Thanks @gethari I'll take a look as soon as I can! 💪

@gethari
Copy link
Contributor Author

gethari commented Jan 23, 2023

@bdbch did you get a chance to check this out ? Please share your thoughts / feedback

@bdbch bdbch self-assigned this Jan 27, 2023
@bdbch bdbch self-requested a review January 27, 2023 14:20
@gethari
Copy link
Contributor Author

gethari commented Feb 13, 2023

@bdbch , I hope this message finds you well. I wanted to inform you that I have recently updated the branch to reflect the latest fork and resolved any conflicts that arose. I would be grateful if you could take a moment to review these changes at your earliest convenience. Thank you!

@bdbch
Copy link
Contributor

bdbch commented Feb 13, 2023

Sure thing. I'll check your PR out as soon as I find some time.

StarterKit.configure({
bulletList: {
keepMarks: true,
keepAttributes: false, // TODO : Making this as `false` becase marks are not preserved when I try to preserve attrs, awaiting a bit of help
Copy link
Contributor

Choose a reason for hiding this comment

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

What exactly happens with marks when list items keep their attributes?

Copy link
Contributor Author

@gethari gethari Feb 19, 2023

Choose a reason for hiding this comment

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

Let's say I have my first line with Purple color and Italic mark applied. When we set keepAttributes to true I am trying to update it with whatever attributes that are there from the previous line. But while doing so, if we have any mark it is not being preserved. But even without keeping the keepAttributes to true the color from the <p> is being preserved on the bullet list.

In this line

 return chain().toggleList(this.name, this.options.itemTypeName, this.options.keepMarks).updateAttributes(ListItem.name, this.editor.getAttributes(TextStyle.name)).run()

image

Copy link
Contributor Author

@gethari gethari Feb 22, 2023

Choose a reason for hiding this comment

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

My doubt is do we need to chain the updateAttributes here in the above mentioned line ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think so - but shouldn't be a problem. Thanks for coming up with more information! I think this should also close #3768

Comment on lines -85 to -86
getAttributes: match => ({ start: +match[1] }),
joinPredicate: (match, node) => node.childCount + node.attrs.start === +match[1],
Copy link

@javan javan Mar 7, 2023

Choose a reason for hiding this comment

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

I believe these removed lines broke support for creating ordered lists with a start offset. I’ve opened an issue with more details: #3831

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@javan great find, I have raised #3833 to resolve this

@gethari gethari deleted the feat-preserve-marks-on-list branch March 8, 2023 02:56
bdbch pushed a commit that referenced this pull request Mar 18, 2023
* fix: #3831

* fix: default attribute + predicate
aliasliao pushed a commit to aliasliao/tiptap that referenced this pull request May 24, 2023
…3541)

* feat: ueberdosis#3540 Ability to preserve marks on lists

* feat: preserveAttrs in list items

* `keepMarks` is working, but need help with `keepAttrs`

* fix: conflict

* avoid casting
aliasliao pushed a commit to aliasliao/tiptap that referenced this pull request May 24, 2023
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.

Ability to preserve marks from previous paragraph when toggling a listItem
3 participants