feat(ui): replace Monaco with CodeMirror #10559
fnetX
patka
Gusted
0ko
viceice
Labels
No labels
arch
riscv64
backport/v1.19
backport/v1.20
backport/v1.21/forgejo
backport/v10.0/forgejo
backport/v11.0/forgejo
backport/v12.0/forgejo
backport/v13.0/forgejo
backport/v14.0/forgejo
backport/v7.0/forgejo
backport/v8.0/forgejo
backport/v9.0/forgejo
breaking
bug
bug
confirmed
bug
duplicate
bug
needs-more-info
bug
new-report
bug
reported-upstream
code/actions
code/api
code/auth
code/auth/faidp
code/auth/farp
code/email
code/federation
code/git
code/migrations
code/packages
code/wiki
database
MySQL
database
PostgreSQL
database
SQLite
dependency-upgrade
dependency
certmagic
dependency
chart.js
dependency
Chi
dependency
Chroma
dependency
citation.js
dependency
codespell
dependency
css-loader
dependency
devcontainers
dependency
dropzone
dependency
editorconfig-checker
dependency
elasticsearch
dependency
enmime
dependency
F3
dependency
ForgeFed
dependency
garage
dependency
Git
dependency
git-backporting
dependency
Gitea
dependency
gitignore
dependency
go-ap
dependency
go-enry
dependency
go-gitlab
dependency
Go-org
dependency
go-rpmutils
dependency
go-sql-driver mysql
dependency
go-swagger
dependency
go-version
dependency
go-webauthn
dependency
gocron
dependency
Golang
dependency
goldmark
dependency
goquery
dependency
Goth
dependency
grpc-go
dependency
happy-dom
dependency
Helm
dependency
image-spec
dependency
jsonschema
dependency
KaTeX
dependency
lint
dependency
MariaDB
dependency
Mermaid
dependency
minio-go
dependency
misspell
dependency
Monaco
dependency
PDFobject
dependency
playwright
dependency
postcss
dependency
postcss-plugins
dependency
pprof
dependency
prometheus client_golang
dependency
protobuf
dependency
relative-time-element
dependency
renovate
dependency
reply
dependency
ssh
dependency
swagger-ui
dependency
tailwind
dependency
temporal-polyfill
dependency
terminal-to-html
dependency
tests-only
dependency
text-expander-element
dependency
urfave
dependency
vfsgen
dependency
vite
dependency
Woodpecker CI
dependency
x tools
dependency
XORM
Discussion
duplicate
enhancement/feature
forgejo/accessibility
forgejo/branding
forgejo/ci
forgejo/commit-graph
forgejo/documentation
forgejo/furnace cleanup
forgejo/i18n
forgejo/interop
forgejo/moderation
forgejo/privacy
forgejo/release
forgejo/scaling
forgejo/security
forgejo/ui
Gain
High
Gain
Nice to have
Gain
Undefined
Gain
Very High
good first issue
i18n/backport-stable
impact
large
impact
medium
impact
small
impact
unknown
Incompatible license
issue
closed
issue
do-not-exist-yet
issue
open
manual test
Manually tested during feature freeze
OS
FreeBSD
OS
Linux
OS
macOS
OS
Windows
problem
QA
regression
release blocker
Release Cycle
Feature Freeze
release-blocker
v7.0
release-blocker
v7.0.1
release-blocker
v7.0.2
release-blocker
v7.0.3
release-blocker
v7.0.4
release-blocker
v8.0.0
release-blocker/v9.0.0
run-all-playwright-tests
run-end-to-end-tests
test
manual
test
needed
test
needs-help
test
not-needed
test
present
untested
User research - time-tracker
valuable code
worth a release-note
User research - Accessibility
User research - Blocked
User research - Community
User research - Config (instance)
User research - Errors
User research - Filters
User research - Future backlog
User research - Git workflow
User research - Labels
User research - Moderation
User research - Needs input
User research - Notifications/Dashboard
User research - Rendering
User research - Repo creation
User research - Repo units
User research - Security
User research - Settings (in-app)
No milestone
No project
No assignees
7 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
forgejo/forgejo!10559
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "beowulf/forgejo-codemirror"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
purpose it acts more like a small IDE than a code editor and is doing
too much. In my limited user research the usage of editing files via
the web UI is largely for small changes that does not need the
features that Monaco editor provides.
by replacing it with Codemirror the amount of time that webpack needs
to build the frontend is reduced by 50% (~30s -> ~15s).
bindatatag) is reduced by 2MiB.less powerful hardware, most notably the lazy loading is much faster
as codemirror uses less javascript.
behavior of the code editor if we wish to.
package.jsonand incodeeditor.tswe have to supply a lot more of its features to havefeature parity with Monaco editor.
an lsp would provide), Codemirror only has such language support to an
extend.
also available in VSCode), this is not available in code mirror.
dynamically changing language support depending on the filename)
still works and the theme is based on the VSCode colors which largely
resembles the monaco editor.
reading how imports are passed around in
codeeditor.ts).found (and fixed) but no major drawbacks were noted for their usage of
the web editor.
on mobile. It is otherwise only accessible via
Ctrl+f.
Co-authored-by: Beowulf beowulf@beocode.eu
Release notes
27f8870042c9ea0f4f34a081ebeb1ac49e53b9040ec80b64dd585b8f9773585b8f97737bef428a78I hate Playwright 😄
7bef428a78a50f53d41fa50f53d41fbc9421eacebc9421eace301c2df378301c2df37886458d952f86458d952f2b851ffd082aa0c6c9c19656da3f229656da3f222a5cd3b15cWhere does that come from?
The following is a preview of the release notes for this pull request, as they will appear in the upcoming release. They are derived from the content of the `release-notes/10559.md` file, if it exists, or the title of the pull request. They were also added at the bottom of the description of this pull request for easier reference.This message and the release notes originate from a call to the release-notes-assistant.
Release notes
2a5cd3b15c08a57ad70b08a57ad70b728da6419fThe e2e tests should be pretty reliable now and locally I have a stable 100% success rate.
@Gusted we can cherry-pick/squash the canges in !8370 or continue with this PR - I let you decide, I'm fine with both. If we keep this PR, I will alter squash the commits, but for now I keep the single commits for easier review.
WIP: feat(ui): replace Monaco with CodeMirrorto feat(ui): replace Monaco with CodeMirrorUnrelated to the technical implementation: I wonder a bit about the overall development (incl. security patches) of CodeMirror when looking at the commit history. It has seen ~ 10 commits in the last 2 years. While there are no open PRs, it seems not much is happening it all, which is somewhat concerning in terms of security and the effort which is being put in to migrate to it.
@pat-s wrote in #10559 (comment):
This repository is not being used to develop code. Take a look at the other packages of codemirror.
Ah, thanks, didn't realize it's split across many repos and
devisn't representative even though it has all the stars.@ -0,0 +1,198 @@// @watch startMissing copyright headers.
@ -0,0 +195,4 @@await page.keyboard.press('ControlOrMeta+F', {delay: 5});await expect(searchField).toHaveCount(0);});There's some logic to switch the language when you change the file extension, this might be something to test a easy one would be that when you rename a file to
.mdthat the "Preview changes" tab is shown.@ -0,0 +1,237 @@import type {SearchQuery} from '@codemirror/search';Ha, I forgot copyright headers myself. Feel free to have this work licensed under GPL-3.0-or-later.
At this point the implementation is pretty solid. Markdown preview needs some styling fixes.
728da6419f67731c016d@0ko wrote in #10559 (comment):
Just some padding around the content, or have you also seen something else?
Well yes, but I needed to asses the root cause, e.g. whether it was caused by some orphaned CSS selectors.
@ -31,2 +15,2 @@.monaco-scrollable-element > .scrollbar > .slider:active {background: var(--color-primary-dark-2) !important;.edit.form .ui.segment {padding: 0;I doesn't seem like this rule is needed.
For preventing unwanted padding in editor tab, there's already this from
fileeditor.cssThe conversion from tabs to switch works good, but need to remove class
attachedfrom segments so they are rounded properly.e822eec6844d8c0b2bd54d8c0b2bd51a0ebf322eResolved both of your comments @0ko & @Gusted
First occurence (after adding them and making them (more) stable) that the e2e test failed randomly. 🤔
Many thanks for getting this over the finish line, the additions looks great.
There's a merge conflict.
@ -34,2 +35,4 @@</div><button class="secondary button" id="editor-find" type="button">{{svg "octicon-search"}}<span class="text not-mobile">Search</span></button></div><div class="ui bottom attached active tab segment" data-tab="write">Remove
attachedso the conversion fromtabular menutoswitchlooks good.@ -41,3 +44,3 @@<div class="editor-loading is-loading"></div>{{template "shared/codemirror_container" .}}</div><div class="ui bottom attached tab segment markup" data-tab="preview">Ditto
Looks mergeable! Some patching work can be done later.
1a0ebf322e200c5452ac86e30db6fd4a324e99e4Removed the
attachedclass at three places (!10559 (commit86e30db6fd)), rebased and resolved the merge conflict and squashed the commits together.The backport to
v14.0/forgejofailed. Check the latest run for more details.❤️
Removing the label because the manual v14 backport will be in the release notes
The backport to
v14.0/forgejofailed. Check the latest run for more details.