[#4051] Use new JSON editor #4407
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Viicos
commented
Jun 18, 2024
|
Not having the ability to specify rows/columns is very inconvenient, I wonder if we should simply support |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4407 +/- ##
=======================================
Coverage 96.57% 96.57%
=======================================
Files 719 719
Lines 24089 24090 +1
Branches 2863 2863
=======================================
+ Hits 23263 23264 +1
Misses 562 562
Partials 264 264 ☔ View full report in Codecov by Sentry. |
Viicos
force-pushed
the
feature/4051-monaco
branch
2 times, most recently
from
d1e1af9
to
d1134be
Compare
sergei-maertens
force-pushed
the
feature/4051-monaco
branch
5 times, most recently
from
0fe115e
to
cba2fd9
Compare
sergei-maertens
force-pushed
the
feature/4051-monaco
branch
from
4524864
to
06182d0
Compare
It takes any json, as primitives or arrays are also valid JSON logic.
Upgraded the builder to a version that uses the same monaco-json-editor version to prevent different library versions being included in the build. This adds the explicit dependency on the monaco editor, instead of relying on it being provided through formio-builder.
These components now use the underlying monaco-json-editor to edit and/or preview the JSON data being manipulated. This fixes the annoying autoformatting-while-typing and adds syntax highlighting, plus support for dark/light theme. Co-authored-by: @Viicos
Sizing is based on number of rows/cols desired and set through a custom CSS property on the respective components.
* Moved around the position of widgets a bit, and declutter * Instead of a toggle link, there's now a toggle icon * Instead of expand/collapse, replace the editor UI with the Json widget and vice versa on toggle * Ensure that icons have accessible labels when a title is provided
Added a helper to put (JSON) expressions in the editors so that the user interaction can be simulated. For speed reasons, it uses copy and paste. Solution is taken from microsoft/playwright#14126
It leads to duplicate labels where the icon is used as a tooltip, instead we can provide the prop/attribute in places where it's actually waranted. This refactors the toggle icon so that the message, behaviour and a11y impact is contained in a single place.
sergei-maertens
force-pushed
the
feature/4051-monaco
branch
6 times, most recently
from
5a35d67
to
4f62faa
Compare
sergei-maertens
force-pushed
the
feature/4051-monaco
branch
5 times, most recently
from
59e7a9b
to
d453cfb
Compare
src/openforms/js/components/admin/form_design/variables/ServiceFetchConfigurationForm.js
Outdated
Show resolved
Hide resolved
src/openforms/js/components/admin/form_design/variables/ServiceFetchConfigurationForm.js
Outdated
Show resolved
Hide resolved
Writing to and reading from monaco editors is a bit more convoluted than making simple assertions on textareas. The helpers should be re-usable for any place that uses the monaco-json-editor.
See microsoft/playwright#13037 for more context.
I could not for the life of me get this to work, after spending 4 hours on it trying various permutations of workarounds: * using a textarea or div that is contenteditable to grab the code from, via page.evaluate. Those actually all worked, but the problem wasn't getting the code/JSON on the clipboard - navigator.clipboard actually works just fine * trying various combinations to get the code in the editor to clear: CTRL+A, CTRL+End (to end of document) followed by CTRL+Shift+Home to select everything from end to beginning of document, followed by Delete key to delete it -> can't get this selection to apply, with or without delays provided in keyboard.press I've added numerous screenshots and compared them to what Chromium browsers do, and that is different - the select-all-and-then-delete to get rid of the existing content just doesn't seem to work. I don't know if the editor is not properly initialized or events are not properly dispatched, but I've given up at this point. So, warning other users via an exception that this won't work on webkit to save them the same debugging hassle and adding a skip helper for these tests should still give us the coverage on chromium and firefox, and manual testing on webkit will have to make do.
sergei-maertens
force-pushed
the
feature/4051-monaco
branch
from
d453cfb
to
07bdb8c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #4424
Changes
Update builder
Use new editor in various places
Checklist
Check off the items that are completed or not relevant.
Impact on features
Release management
I have updated the translations assets (you do NOT need to provide translations)
./bin/makemessages_js.sh./bin/compilemessages_js.shCommit hygiene