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

Add Monaco editor #9

Merged
merged 13 commits into from Dec 4, 2023
Merged

Add Monaco editor #9

merged 13 commits into from Dec 4, 2023

Conversation

vemonet
Copy link
Contributor

@vemonet vemonet commented Nov 29, 2023

Hi @pchampin , about #4 I tried running the Monaco editor using the ESM import, and it worked without any issue! (the problem I faced in my app was due to a HighlightJS import conflict). The API to manipulate the Monaco editor is quite straightforward, so I went ahead and updated the app:

  • Use the Monaco editor for input and output, it works exactly like before, but now uses the Monaco editor instead of textarea
  • Enabled automatic language change when the input/output format changes. It switches between xml, json, and sparql (works for every w3c syntax: nt, turtle, quads...)
  • Used the solarized-dark theme from https://github.com/brijeshb42/monaco-themes/tree/master/themes
  • Changed the layout to use flexbox, center items, and make it responsive: on medium and large screen the input and output editor will be displayed side by side, on small screen they will be displayed on top of each other
  • Added the w3c RDF logo as SVG to use as favicon (feel free to change if you have a better idea in mind)

There is still the placeholder in the input editor to implement, I added a comment with a pointer on how to do it

Here is what it looks like with the solarized-dark theme:

Screenshot from 2023-11-29 11-37-06

And with the solarized-light theme:

Screenshot from 2023-11-29 11-39-19

Let me know which changes you would like to do next. I am thinking about this:

  • increase the height of the editors depending on screen size
  • reduce the size of the title to gain space for the rest, and make it more informative. Something like "SoWasm - RDF playground based on Sophia: validate, convert, canonicalize" on 1 line

Copy link
Owner

@pchampin pchampin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for this.

I wish you had split the different changes (improve HTML+CSS, add Monaco editor) in separates PRs, but now that it is done, I will not ask you to redo the work.

I suggested a few changes, some of them pure linting, but one of them is actually a regressop,: the input editor was previously made readable once the URL had been loaded, and this is not the case anymore.

demo/script.js Outdated Show resolved Hide resolved
demo/script.js Outdated Show resolved Hide resolved
demo/script.js Outdated Show resolved Hide resolved
demo/script.js Outdated Show resolved Hide resolved
vemonet and others added 6 commits November 29, 2023 14:01
Co-authored-by: Pierre-Antoine Champin <github-100614@champin.net>
Co-authored-by: Pierre-Antoine Champin <github-100614@champin.net>
Co-authored-by: Pierre-Antoine Champin <github-100614@champin.net>
Co-authored-by: Pierre-Antoine Champin <github-100614@champin.net>
…or is always readonly, we don't need to enable/disable it
@vemonet
Copy link
Contributor Author

vemonet commented Nov 29, 2023

Sorry, I thought separating it in different commits would be enough, and I needed to have the Monaco editors setup to properly do the work on the CSS display

I also removed some leftovers that were not used anymore

Copy link
Contributor Author

@vemonet vemonet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look good to me, just I am not sure I understand the situation with the input editor being set readonly when using a URL to load the RDF

When I check your original version after loading a URL the editor is still editable. Currently how we implemented in this new version the input editor is set to readonly if a URL is provided

@pchampin
Copy link
Owner

Look good to me, just I am not sure I understand the situation with the input editor being set readonly when using a URL to load the RDF

When I check your original version after loading a URL the editor is still editable. Currently how we implemented in this new version the input editor is set to readonly if a URL is provided

The editor is set to read-only only while the URL is loading. The goal was two-fold

  • change the style of the textarea (ligher text) to make it clear that "(loading)" was a message rather than an input
    (but I guess that's moot, I don't think that Monaco changes its style when read-only)
  • to prevent the user to type something that would be lost once the content is loaded

@vemonet
Copy link
Contributor Author

vemonet commented Nov 30, 2023

Ok, I fixed it

@vemonet
Copy link
Contributor Author

vemonet commented Nov 30, 2023

I also have the code ready to add inline error highlighting in the editor :) It works perfectly for syntax errors I tried in trig/turtle/nt/nq!

For JSON-LD we get only a position without line, I suspect this is a position after removing newlines and extra spaces, so this might need additional work to get it to work perfectly. That said the built-in JSON validation already helps to pick up all regular JSON errors

Not sure if I should wait for this PR to be merged to create a new one, or if I can directly push to this PR? It's only adding 1 function ~10 lines

@pchampin
Copy link
Owner

pchampin commented Dec 1, 2023

I also have the code ready to add inline error highlighting in the editor :) It works perfectly for syntax errors I tried in trig/turtle/nt/nq!

nice! :)

For JSON-LD we get only a position without line, I suspect this is a position after removing newlines and extra spaces, so this might need additional work to get it to work perfectly. That said the built-in JSON validation already helps to pick up all regular JSON errors

Not sure if I should wait for this PR to be merged to create a new one, or if I can directly push to this PR? It's only adding 1 function ~10 lines

I guess it is small enough to include, then

@pchampin
Copy link
Owner

pchampin commented Dec 1, 2023

I thought I had written this comment already, but I must have forgot to press 'Comment'???

There are currently two issues that are really bothering me:

  • when pressing 'tab' in the Monaco editor, it... inserts a tab ;
    contrast with the textarea, where 'tab' allows to focus the next element of the page.
    This is important for accessibility.

  • the Monaco editor does not emit the 'input' event in the following situations :

    • when you press delete or backspace,
    • when you cut or paste text.

    This breaks the good working of the 'automatic' mode.

As much as I appreciate and like the work you have been doing, those are really blocking issues for me at this point :-/

@vemonet
Copy link
Contributor Author

vemonet commented Dec 1, 2023

You can still toggle between the default tab behavior and adding tabs by hitting Ctrl + m, we could add a small note to let people know. But I guess you would prefer to have the default behavior by default, so I added a fix to enable this (but we cannot use ctrl+m to go back to "adding tabs" behavior anymore)

I also updated the listener to use the builtin onInputChange event of the monaco editor, so it properly updates at every change now

@pchampin pchampin merged commit cd8a081 into pchampin:main Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants