Improved JSON viewing/editing with Ace editor

[hogpilot]

This PR includes a new integration of Ace Editor for viewing and editing JSON. Highlight.js was not meant for editing, and the previous line number integration didn't work very well when adding/deleting lines. Here are things to test:

• Fixes Copying Full JSON into Data section does not "work" anymore #212
• Fixes an unlogged bug where multiple update requests were being sent when the same item was saved multiple times
• New capability to expand/collapse JSON
• New logic for tooltip integration with the associated JSON buttons (copy/save/edit).
• New capability to find/replace JSON text (Ctrl+F)

For now, a build of this PR is available at hogpilot/ago-assistant

[hogpilot]

I did not remove js\lib\highlight-8.3.min.js (even though it is not used), thoughts?

[bsvensson]

The new code editor looks and works great :)

As far as the PR goes, I think most of those ace files are not needed. For example, we're only editing json files (not all file types).

[hogpilot]

@bsvensson I agree, and only found 4 files used in my testing:

• js/lib/ace/ace.js
• js/lib/ace/mode-json.js
• js/lib/ace/theme-tomorrow.js
• js/lib/ace/worker-json.js

I left the rest of them in there in case somewhere down the line, other ace options are utilized. I went back and forth about the proper approach for this scenario, but don't feel strongly one way or the other. For now, the full ace library source code is in there...

I'm open for thoughts on which approach to take. The current approach is safe, and while it increases the repo and deployment size, it has no impact on the client network traffic. I was afraid some other ace call might break things down the line if I removed a file that didn't come up in my testing.

[hogpilot]

Case and point, Find/Replace (Ctrl-F) triggers an additional file (thanks @bsvensson , i didn't know you could do that):

• js/lib/ace/ext-searchbox.js

[hogpilot]

Another note on which Ace files to include. Pressing Ctrl+, brings up a settings menu. You can change options/themes there, which will dynamically load additional ace files. It might be nice to leave that feature (give a try to see what I mean), which might not allow us to trim down the ace files. Or, we could just include the 5 or 6 files that @bsvensson identified and disable the settings menu.

I'm open to feedback if anyone feels strongly one way or another.

[slibby]

@hogpilot seeing a few strange characters on one item - highlighted in red:

[hogpilot]

@slibby interesting. Ace is telling you that there is an invalid or invisible character there. It turns out, this is because the JSON contains \u00A0 (or &nbsp, or character code 160) in the text. This is due to the ArcGIS Online WYSIWYG editor adding a non-breaking space.

Since this is not printable (other than as a space), Ace is differentiating it for you. I agree that red is not ideal, so I can override the CSS. How does this look?

[hogpilot]

@slibby @bsvensson thanks for the feedback. The latest commit includes the changes you mentioned:

• Remove unused Ace files
• Disable Ace settings menu Ctrl+,
• Switch theme to dark background when editing
• Change CSS for &nbsp character so it is not red

I think this is ready once testing is complete (@slibby? @rwmajor2? @ecaldwell?), let me know if you have questions.

[hogpilot]

Can someone else test https://hogpilot.github.io/ago-assistant/build/ in IE to see if they get weird cursor/text-selection behavior? If so, anybody have an idea of what CSS is the culprit? Thanks.

[bsvensson]

Can someone else test https://hogpilot.github.io/ago-assistant/build/ in IE to see if they get weird cursor/text-selection behavior?

Yes, the cursor becomes the cursor-move cursor after clicking the edit/pen icon to go into Edit mode. Weird.

Double-clicking to select text does the right thing (functionally), but displays a selection box of sorts in the wrong place. Weird.

I don't see the same issue in IE11 for neither the JSAPI sandbox, nor the Ace kitchen sink.

If so, anybody have an idea of what CSS is the culprit?

Nope :(

[hogpilot]

@bsvensson Thanks for checking. I found the culprit and fixed it in the latest commit. It had to do with our use of contenteditable attribute as it pertains to the Enter keypress. Apparently, IE didn't like me using that attribute on a div and when I added it to the Ace editor during edits, it threw off the CSS. Fix was to stop using the contenteditable attribute and instead rely on the .getReadOnly() operation of Ace.

[hogpilot]

Note: In my latest commit, I also snuck a few changes into templates.html that I found while searching for IE issue. These changes are not related to the PR, other than the fact that they are obvious errors that should be fixed (my opinion at least).

[ecaldwell]

I also snuck a few changes into templates.html that I found

Nice find! Maybe this could be addressed with better linting.

[setldsolutions]

Without reading through all this: are we saying #212 and #213 are fixed (since both threads are closed)? E.g. the pasting of JSON back from notepad can now deal with the window's requirement for numbered lines? If so, that's great.

However, there is still another related issue. Simply hitting the pencil, so that I can click in the box, CTRL A, CTRL C and CTRL V into Notepad ++ takes wayyyyyyyyy longer than it used to. It used to take maybe 30 seconds to give me access to the text - now, it takes 15 browser waits and maybe 30 minutes.

[slibby]

@setldsolutions yes, both of those should be fixed in the latest code here on github, which has not yet been deployed to https://ago-assistant.esri.com. In the interim, you can try this build of the app first and see if you still have that delay: https://hogpilot.github.io/ago-assistant/build/ -- we are working on getting the latest build deployed.

[setldsolutions]

Thanks Sam.

[setldsolutions]

Confirmed working - no hitting the pencil to start editing necessary.

Simply click in the box, wait a sec, CTRL A, CTRL C, CTRL V to Notepad ++ --- 91k+ lines of JSON in 2 seconds.

Thanks again!