Pass a Markdown serializer to the Content Editor
What does this MR do?
This MR Refactors the Content Editor to decouple Markdown serializing from the Editor itself. The goal is allowing to customize Markdown serialization by configuring a separate serialization object independently from the Content Editor itself.
What are the serialization/deserialization targets?
When deserializing, we want to convert a Markdown string into a ProseMirror document and serialization performs the inverse operation.
Why is this useful?
We have to parse and render Markdown before converting it into a ProseMirror document. The process of parsing and rendering Markdown varies across use cases. That’s why we want to decouple this operation from the Content Editor.
Screenshots (strongly suggested)
This MR does not introduce user-facing changes. The Content Editor is not used by any feature yet.
Does this MR meet the acceptance criteria?
Conformity
-
Does this MR need a changelog?-
I have included a changelog entry. -
I have not included a changelog entry because this MR does not introduce user-facing changes.
-
-
Documentation(if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Merge request reports
Activity
changed milestone to %13.11
added frontend groupeditor [DEPRECATED] labels
- Resolved by Denys Mishunov
1 Warning Please add a merge request type to this merge request. 1 Message CHANGELOG missing: If you want to create a changelog entry for GitLab FOSS, run the following:
bin/changelog -m 58678 "Pass a Markdown serializer to the Content Editor"
If you want to create a changelog entry for GitLab EE, run the following instead:
bin/changelog --ee -m 58678 "Pass a Markdown serializer to the Content Editor"
If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.
Reviewer roulette
Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category (e.g. frontend or backend), and by a maintainer in all other categories.
To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
Category Reviewer Maintainer frontend Coung Ngo ( @cngo
) (UTC+1, 5 hours ahead of@ealcantara
)Simon Knox ( @psimyn
) (UTC+10, 14 hours ahead of@ealcantara
)If needed, you can retry the
danger-review
job that generated this comment.Generated by
DangerEdited by 🤖 GitLab Bot 🤖- Resolved by Denys Mishunov
@himkp: Could you review this MR, please? I decided to stick with the serializer idea. I know that we won’t support serialization targets anytime soon, but the concept is still useful for several reasons:
- We can group the two operations (to markdown/from markdown) in a single place. This put closely related responsibilities together which increases cohesiveness.
- It gives us a more consistent API because deserialize/serialize have similar return types and parameters but inverted.
- We should expose the concept of Markdown Serialization to the features using the Content Editor because they have to configure it by providing a custom render function.
Bundle size analysis [beta]
This compares changes in bundle size for entry points between the commits 2895caf8 and a3132e46
Special assetsEntrypoint / Name Size before Size after Diff Diff in percent average 3.15 MB 3.15 MB - 0.0 % mainChunk 1.87 MB 1.87 MB - 0.0 %
Note: We do not have exact data for 2895caf8. So we have used data from: 395a3f53.
The target commit was too new, so we used the latest commit from master we have info on.
It might help to rerun thebundle-size-review
job
This might mean that you have a few false positives in this report. If something unrelated to your code changes is reported, you can check this comparison in order to see if they caused this change.Please look at the full report for more details
Read more about how this report works.
Generated by
DangerEdited by 🤖 GitLab Bot 🤖Setting label(s) devopscreate sectiondev based on ~"group::editor".
added devopscreate sectiondev labels
- Resolved by Enrique Alcántara
- Resolved by Denys Mishunov
@dmishunov: Could you mantainerize this MR, please?
requested review from @dmishunov and removed review request for @himkp
added 1 commit
- e921a1b3 - Provide a Markdown renderer to the Content Editor
added 363 commits
-
e921a1b3...63ddf132 - 362 commits from branch
master
- d1e89d3c - Provide a Markdown renderer to the Content Editor
-
e921a1b3...63ddf132 - 362 commits from branch
marked the checklist item Code review guidelines as completed
marked the checklist item Merge request performance guidelines as completed
marked the checklist item Style guides as completed
marked the checklist item Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. as completed
marked the checklist item Style guides as incomplete
marked the checklist item Merge request performance guidelines as incomplete
marked the checklist item Code review guidelines as incomplete
marked the checklist item Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. as incomplete
marked the checklist item Code review guidelines as completed
marked the checklist item Style guides as completed
- Resolved by Enrique Alcántara
- Resolved by Denys Mishunov
- Resolved by Denys Mishunov
- Resolved by Denys Mishunov
added 402 commits
-
d1e89d3c...409ada30 - 401 commits from branch
master
- d2ab08a7 - Provide a Markdown renderer to the Content Editor
-
d1e89d3c...409ada30 - 401 commits from branch
mentioned in issue create-stage#12819 (closed)
- Resolved by Denys Mishunov
- Resolved by Denys Mishunov
@ealcantara thanks a lot for all the responses here _ makes my understanding of this editor a bit better. I have left just one small nitpick that would be great to cover in this MR and we should be ready to merge it! Great job!
added 324 commits
-
d2ab08a7...51cdbf22 - 322 commits from branch
master
- eeb7897f - Provide a Markdown renderer to the Content Editor
- fe14f559 - Code review feedack
-
d2ab08a7...51cdbf22 - 322 commits from branch
enabled an automatic merge when the pipeline for 708cf420 succeeds
added 69 commits
-
fe5be01c...2d0d23ab - 67 commits from branch
master
- 484114c5 - Provide a Markdown renderer to the Content Editor
- a3132e46 - Code review feedack
-
fe5be01c...2d0d23ab - 67 commits from branch
enabled an automatic merge when the pipeline for 4b021d9a succeeds
mentioned in commit e61d74af
added workflowstaging label