fregante / GhostText

👻 Use your text editor to write in your browser. Everything you type in the editor will be instantly updated in the browser (and vice versa).
https://GhostText.fregante.com
MIT License
3.25k stars 116 forks source link

Support fields nested in CodeMirror widgets #203

Closed charness closed 3 years ago

charness commented 3 years ago

Bitbucket Server pull requests use CodeMirror for code display and textareas for line-comment widgets.

Check the field for an enclosing .CodeMirror-linewidget and skip the CodeMirror hook-up when found. Without this change, editing a comment instead pulls in the source code, and saving overwrites the markup on the source. (Even with this change, editing the source itself wipes out the widget.)

Extend the CodeMirror example in the demo so that clicking on a linenumber toggles a textarea line widget into and out of existence. The textarea as a widget reproduces the buggy behavior in the version of the plugin before this commit.

charness commented 3 years ago

(My force-push was due to a lint error on the PR. A mention of clean lint in contributing.md may be helpful.)

fregante commented 3 years ago

Thanks for the PR! It’s really strange that they’re using an editor just to display code. Is the code immediately editable? for example by double-clicking.

Anyway, the PR code exclusively supports textareas only, while I suppose it’s possible that it could as well be another CodeMirror instance or contentEditable. I think the code should be something like:

const cm = closest(CodeMirror, CodeMirror-widget)
if (cm && !cm.matches(CodeMirror-widget)) {

A comment is needed to explain why this code is needed, e.g. “If the field is in a CodeMirror widget, then it’s unrelated to CodeMirror.”

fregante commented 3 years ago

As for the demo, the code is being transferred to the mini site https://github.com/fregante/ghosttext.fregante.com/pull/7, but it’s not done yet.

Please don’t force push between reviews, the commits don’t matter in my PRs anyway, but they’re useful to understand what changed while reviewing.

Also most projects should have lint, so it should be kind of expected.

charness commented 3 years ago

Yes, that's a much better approach to stopping at the closer of a CodeMirror or a widget in one.

I updated the demo mainly to reproduce the buggy behavior in case you wanted to work on a different fix than what I supplied here. Happy to take that change back out of this PR if you don't want it with the code change. Let me know.

No, the code isn't editable in the Bitbucket PR. Atlassian tend to do a lot with their products I don't quite understand....

Sorry about the first force-pushes. I'm more used to closed development in a central repo, where the commits stick around and matter. (The second was no content change, just a switch of author & committer from my personal email to the GitHub one.)

fregante commented 3 years ago

No worries, thank you for the change.

Did you verify it works? I'm going to merge it soon, but I'll exclude the demo for now.

charness commented 3 years ago

Yes, I've verified it works locally in npx web-ext run against all the fields in the demo and against Bitbucket itself.

Thanks for the extension. When Bitbucket's odd structure broke your latest release, I needed to get back to GhostText working ASAP!

fregante commented 3 years ago

I just added the testing page to the minisite: https://ghosttext.fregante.com/test/

Can you send a PR to this file? https://github.com/fregante/ghosttext.fregante.com/blob/main/test/index.md