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

Use Monaco widget for text editors #24

Closed
fsteeg opened this issue May 5, 2021 · 10 comments · Fixed by #28
Closed

Use Monaco widget for text editors #24

fsteeg opened this issue May 5, 2021 · 10 comments · Fixed by #28

Comments

@fsteeg
Copy link
Member

fsteeg commented May 5, 2021

For the Date, Flux, Fix text areas

@katauber
Copy link
Member

katauber commented Jun 9, 2021

Deployed to http://test.lobid.org/playground/
Exchanged the texareas for data, flux and fix with monaco editor widgets. The inputs aren't resizeable anymore. So please check if they're big enough.

@katauber katauber assigned acka47 and TobiasNx and unassigned katauber Jun 9, 2021
@TobiasNx
Copy link
Contributor

TobiasNx commented Jun 9, 2021

This looks good in my opinion. ctrl+enter works too.

One minor thing the editor has a preview could this be deleted since it is not necessary in my opinion.

@acka47
Copy link
Contributor

acka47 commented Jun 9, 2021

This adds some nice features.
I played around with it a bit, here are my comments:

  • The initial state of the web page confuses me a bit. This is due to there only being one line per window which is highlighted. It would be good to see empty lines with line numbers in all input fields. It's also be nice to have the cursor in the "data" input field after page load.
  • The initial sizing of the windows looks good to me but eight rows in the "Data" window are not enough when adding a file with >100 lines. Wouldn't it generally be possible to resize the window according to the input to something like 1/3 of lines in the input file? (I don't know what size is best here, we have to try out.)
  • I am not sure about the document overview at the right-hand side of the fix and flux window (what is the technical term for this overview). It all looks a bit squeezed with the two windows side by side. But we can continue with this setup for now.
  • Have you checked the contrast wrt accessibility? The colour of the selected line number has very little contrast to the input text as well as to the numbers of the not selected lines.

As a general remark, testing would be easier and the results probably better, if we had more diverse examples. We have #25 for this, which is assigned to @fsteeg .

@acka47 acka47 assigned katauber and unassigned acka47 and TobiasNx Jun 9, 2021
@katauber
Copy link
Member

katauber commented Jun 9, 2021

The initial state of the web page confuses me a bit. This is due to there only being one line per window which is highlighted. It would be good to see empty lines with line numbers in all input fields. It's also be nice to have the cursor in the "data" input field after page load.

There only are line numbers when there is content. I think that cannot be changed only with a trick like having content which contains multiple line breaks, but I don't like that idea. Maybe it's a better way to start with an example instead of empty editors?
I will fix that the cursor will be in data-editor after page load.

  • The initial sizing of the windows looks good to me but eight rows in the "Data" window are not enough when adding a file with >100 lines. Wouldn't it generally be possible to resize the window according to the input to something like 1/3 of lines in the input file? (I don't know what size is best here, we have to try out.)

That's possible. It would be nice to have some examples for that. Maybe we can implement resizing of the data-editor at the same time we offer more examples?
Resizing of the other editors (by the user) should be possible , too, but as we discussed offline we wanted to do this in an own issue.

One minor thing the editor has a preview could this be deleted since it is not necessary in my opinion.

I am not sure about the document overview at the right-hand side of the fix and flux window (what is the technical term for this overview). It all looks a bit squeezed with the two windows side by side. But we can continue with this setup for now.

The so called mini-map comes as default with the monaco editor, but can be disabled. I think the mini-map will be useful when there are bigger files and when we have fix and flux language support, because you can identify really fast problems or violations via the mini-map and jump to this point. I will disable it for now, but think we should enable it again later.

Have you checked the contrast wrt accessibility? The colour of the selected line number has very little contrast to the input text as well as to the numbers of the not selected lines.

I did the lighthouse checks but it seems that they cannot analyze the editors. We already have low contrast ratio for all buttons. Last time we discussed that it's no a great problem.
For the editors we can choose between a light and dark theme or create our own theme. I thought the dark theme did not match with the rest of the page.
Creating an own theme should be in a new issue I think.

@acka47
Copy link
Contributor

acka47 commented Jun 9, 2021

I think that cannot be changed only with a trick like having content which contains multiple line breaks, but I don't like that idea.

I was afraid it is like that and I don't like the trick as well.

Maybe it's a better way to start with an example instead of empty editors?

I am not sure about this. Please leave it empty for now.

I will fix that the cursor will be in data-editor after page load.

Good

Maybe we can implement resizing of the data-editor at the same time we offer more examples?

+1

I will disable it for now, but think we should enable it again later.

+1

Creating an own theme should be in a new issue I think.

Ok, then let's discuss this when the basic functionality is there.

katauber added a commit that referenced this issue Jun 10, 2021
katauber added a commit that referenced this issue Jun 10, 2021
katauber added a commit that referenced this issue Jun 10, 2021
katauber added a commit that referenced this issue Jun 10, 2021
@katauber
Copy link
Member

Deployed again to http://test.lobid.org/playground/

@katauber katauber assigned acka47 and TobiasNx and unassigned katauber Jun 10, 2021
@acka47
Copy link
Contributor

acka47 commented Jun 11, 2021

+1

@acka47 acka47 removed their assignment Jun 11, 2021
@TobiasNx
Copy link
Contributor

I like this without the mini-preview a lot better. +1

grafik

Is the line at the end of the editor intentional? This was hidden behind the mini-preview I think.

@TobiasNx TobiasNx assigned katauber and unassigned TobiasNx Jun 11, 2021
@katauber
Copy link
Member

katauber commented Jun 11, 2021

Is the line at the end of the editor intentional? This was hidden behind the mini-preview I think.

Not really. This comes with the Monaco Editor. The line is the border for the scroll bar. It seems that the scrollbar is put over the selected line border instead being beside the selected line, when the minimap is disabled. Since I found no hot-fix for this, can we move this task to be part of #29 @TobiasNx ?

@TobiasNx
Copy link
Contributor

+1

@TobiasNx TobiasNx assigned fsteeg and unassigned katauber Jun 11, 2021
@dr0i dr0i unassigned fsteeg Jun 14, 2021
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 a pull request may close this issue.

4 participants