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

Remove easymde, tributejs and codemirror #30218

Closed
wants to merge 2 commits into from

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Mar 31, 2024

WIP because the wiki editor is broken in various ways. Other editors should work as usual.

This also introduces a migration mechanism for localStorage, in this case to get rid of two unused keys.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 31, 2024
@silverwind silverwind marked this pull request as draft March 31, 2024 18:09
@pull-request-size pull-request-size bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 31, 2024
@silverwind
Copy link
Member Author

Maybe I will implement side-by-side mode that was previously only active on wiki which gets lost with this removal.

If I understand correctly, this mode was not a accurate markdown rendering because it was produced by EasyMDE instead of the backend renderer, so the new solution would definitely make use of the backend renderer, e.g. the same one that is used for the preview tab in issues/prs.

@wxiaoguang
Copy link
Contributor

I strongly prefer do not do so until it is clear that there is no user uses EasyMDE. Although it is not modern, it does help some non-technical background people.

And I have objection for removing the "combo" prefix, the name was chosen intentionally, to distinguish it from other "markdown editors". And it is indeed a real "combo": it has toolbar, it works with uploader, etc.

@silverwind
Copy link
Member Author

silverwind commented Apr 2, 2024

I strongly prefer do not do so until it is clear that there is no user uses EasyMDE

There were zero complaints when we switched the default editor and it should be noticable because EasyMDE users see a flash of wrong content on every page load. So I think it's high time to get rid of it.

And I have objection for removing the "combo" prefix

I thought the "combo" refered to it being two editors in one. Still prefer it without the prefix because the toolbar is part of the editor in my view, otherwise it would just be "textarea".

@silverwind
Copy link
Member Author

Well, looks like no one here is interested cleaning up this legacy stuff it seems.

@silverwind silverwind closed this Apr 13, 2024
@delvh
Copy link
Member

delvh commented Apr 13, 2024

Oh, there is, I simply forgot about this PR like so many others on my bucket list.

@silverwind
Copy link
Member Author

I would stil aim to remove it for 1.23. Anyone agree?

@delvh
Copy link
Member

delvh commented Apr 14, 2024

Yep, I agree. I'll go ahead and post an issue to gather community feedback.
If there is no heavy feedback not to remove it, then we should remove it for 1.23.

@silverwind silverwind reopened this Apr 14, 2024
@delvh
Copy link
Member

delvh commented Apr 14, 2024

See #30474

@silverwind
Copy link
Member Author

silverwind commented Apr 14, 2024

I think I would have a fresh go at another time as #30480 will introduce too many conflicts here. @wxiaoguang what is your preference on the name? Keep ComboMarkdownEditor? I find that an awefully long name.

@silverwind silverwind closed this Apr 14, 2024
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Apr 14, 2024

I think I would have a fresh go at another time as #30480 will introduce too many conflicts here. @wxiaoguang what is your preference on the name? Keep ComboMarkdownEditor? I find that an awefully long name.

And I have objection for removing the "combo" prefix, the name was chosen intentionally, to distinguish it from other "markdown editors". And it is indeed a real "combo": it has toolbar, it works with uploader, etc.

I do like long names, it doesn't make anything worse, it doesn't slow down, it doesn't waste storage. It indeed makes the code clear, conflict-free and maintainable. We can search anything from whole codebase easily.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/dependencies modifies/docs modifies/internal modifies/js modifies/templates This PR modifies the template files modifies/translation size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants