nicegui icon indicating copy to clipboard operation
nicegui copied to clipboard

`ui.codemirror` not updating when the bound dictionary changes

Open zkx06111 opened this issue 1 year ago • 3 comments

Description

Using nicegui==1.4.28

from nicegui import ui

shared_memory = {
    'code': '// Write your code here',
    'design': '',
    'test': '',
}


@ui.page('/')
def main():
    editor = ui.codemirror(language='python').bind_value(shared_memory, 'code')
    ui.markdown(shared_memory["code"]).bind_content(shared_memory, 'code')
    ui.button('clear code').on('click', lambda: shared_memory.update(code=''))
    
ui.run()

When the button is clicked the first time, it works as expected. Both the markdown text and the codemirror editor are cleared.

However, when it's clicked the second time, the markdown gets cleared while the codemirror remains the same.

When I try to edit what's in codemirror, I get the following error.

Traceback (most recent call last):
  File "/Users/kexunz/miniconda3/envs/ad/lib/python3.11/site-packages/nicegui/events.py", line 413, in handle_event
    result = handler(arguments) if expects_arguments else handler()
             ^^^^^^^^^^^^^^^^^^
  File "/Users/kexunz/miniconda3/envs/ad/lib/python3.11/site-packages/nicegui/elements/mixins/value_element.py", line 40, in handle_change
    self.set_value(self._event_args_to_value(e))
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/kexunz/miniconda3/envs/ad/lib/python3.11/site-packages/nicegui/elements/codemirror.py", line 294, in _event_args_to_value
    new_value = changeset.apply(self.value)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/kexunz/miniconda3/envs/ad/lib/python3.11/site-packages/nicegui/elements/codemirror.py", line 322, in apply
    raise ValueError('Cannot apply change set to a document with the wrong length')
ValueError: Cannot apply change set to a document with the wrong length

zkx06111 avatar Jul 10 '24 01:07 zkx06111

It is a little similar to #3217 . I change LOOPBACK = None in .venv\Lib\site-packages\nicegui\elements\codemirror.py to LOOPBACK = True. It works as expected.

Following code shows that _props['value'] has changed, but the value displayed in ui.codemirror is the same as the value before.

from nicegui import ui

shared_memory = {
    'code': '',
    'design': '',
    'test': '',
}

@ui.page('/')
def main():
    editor = ui.codemirror(language='python').bind_value(shared_memory, 'code')
    ui.markdown(shared_memory["code"]).bind_content(shared_memory, 'code')
    ui.button('clear code').on('click', lambda: shared_memory.update(code=''))
    ui.button('print',on_click=lambda :ui.notify(editor._props['value']))

ui.run()

python-and-novella avatar Jul 10 '24 04:07 python-and-novella

Thanks for reporting this bug, @zkx06111! It is indeed very similar to #3217. Therefore I'll close this as duplicate and add a ui.codemirror example over there.

falkoschindler avatar Jul 10 '24 09:07 falkoschindler

It turned out that fixing ui.editor doesn't fix ui.codemirror, even though both problems are very similar. Therefore I'll re-open this issue.

Minimum reproduction:

code = ui.codemirror(value='foo')
ui.label().bind_text_from(code, 'value')
ui.button('Set X', on_click=lambda: code.set_value('X'))

The button only works once.

Somehow we need a similar solution like PR #3346, but combined with the already existing Vue component in codemirror.js.

falkoschindler avatar Jul 12 '24 10:07 falkoschindler

Hi,

I'm on Win11, with NiceGUI 2.11.0.

Got the same by using this simple code :

from nicegui import events, ui

with ui.dialog().props('full-width') as dialog:
    with ui.card():
        content = ui.codemirror()


def handle_upload(e: events.UploadEventArguments):
    text = e.content.read().decode('utf-8')
    content.set_value(text)
    dialog.open()

ui.upload(on_upload=handle_upload).props('accept=.py').classes('max-w-full')

ui.run()

The error :

Cannot apply change set to a document with the wrong length Traceback (most recent call last): File "C:\Users\zak-4\PycharmProjects\WLEDVideoSync.venv\Lib\site-packages\nicegui\events.py", line 430, in handle_event result = cast(Callable[[EventT], Any], handler)(arguments) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\Users\zak-4\PycharmProjects\WLEDVideoSync.venv\Lib\site-packages\nicegui\elements\mixins\value_element.py", line 40, in handle_change self.set_value(self._event_args_to_value(e)) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\Users\zak-4\PycharmProjects\WLEDVideoSync.venv\Lib\site-packages\nicegui\elements\codemirror.py", line 335, in _event_args_to_value new_value = changeset.apply(self.value) ^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "C:\Users\zak-4\PycharmProjects\WLEDVideoSync.venv\Lib\site-packages\nicegui\elements\codemirror.py", line 363, in apply raise ValueError('Cannot apply change set to a document with the wrong length') ValueError: Cannot apply change set to a document with the wrong length

zak-45 avatar Feb 05 '25 10:02 zak-45

does anyone find out solution? modify LOOPBACK = None in .venv\Lib\site-packages\nicegui\elements\codemirror.py to LOOPBACK = True just got a part solution, as said the value displayed in ui.codemirror is the same as the value before.

cipologic avatar Mar 26 '25 07:03 cipologic

Hey!

~~Isn't this the same issue as #4575 which is to be fixed via #4578?~~

~~The ValueError: Cannot apply change set to a document with the wrong length totally gives it away!~~

~~And yeah, in comment https://github.com/zauberzeug/nicegui/issues/3337#issuecomment-2636378576:~~

~~You are reading the user upload via text = e.content.read().decode('utf-8')~~

~~Should any of the characters inside text take more than 2 codepoints to store in UTF-16, the code will blow up.~~

No it isn't. But the true fix is at #4586. https://github.com/zauberzeug/nicegui/issues/3337#issuecomment-2636378576 just happens to need both #4578 and #4586 to fix completely.

evnchn avatar Apr 07 '25 02:04 evnchn

@evnchn In https://github.com/zauberzeug/nicegui/issues/3337#issuecomment-2225327533 I'm not using any special characters. So I don't see why the encoding is relevant 🤔

falkoschindler avatar Apr 07 '25 05:04 falkoschindler

@evnchn In #3337 (comment) I'm not using any special characters. So I don't see why the encoding is relevant 🤔

Hmm 🤔 let me check that out real soon. But the UTF-16 fix should be valid in its own right.

evnchn avatar Apr 07 '25 05:04 evnchn

@falkoschindler I got a temporary fix for https://github.com/zauberzeug/nicegui/issues/3337#issuecomment-2225327533

Core issue: You're not supposed to simply write to the "value" prop of the codemirror element.

Rather, you need to do setEditorValue so that the CodeMirror JavaScript side can be aware of the updated value.

from nicegui import ui 

@ui.page('/')
def main_page():
    code = ui.codemirror(value='foo')
    ui.label().bind_text_from(code, 'value')
    ui.button('Set X', on_click=lambda: code.set_value('X')).on('click', js_handler=f'() => {{ getElement({code.id}).setEditorValue("X") }}')

ui.run(port=8080)

evnchn avatar Apr 07 '25 05:04 evnchn

Image Image

Yep, so by running setEditorValue, it makes CodeMirror aware, hey, someone gave me new value, and so the subsequent changeset applies success.

Unrelated to #4578. Sorry for jumping the gun. Will PR ASAP.

evnchn avatar Apr 07 '25 06:04 evnchn

@evnchn That's a very interesting insight! But my main question is:

Why does it work for the first time?

code = ui.codemirror(value='foo')
ui.button('Set X', on_click=lambda: code.set_value('X'))

Clicking the button will set the editor value to "X".

The plan was to watch the "value" prop and call setEditorValue automatically: https://github.com/zauberzeug/nicegui/blob/03ceeb17ec325e3955bfa8bfa368eda0fadc53d4/nicegui/elements/codemirror.js#L15-L18 Why doesn't this work?

If we need to change how the value is updated, we might as well need to handle language, theme and disable differently, as you already pointed out.

falkoschindler avatar Apr 07 '25 15:04 falkoschindler

Why doesn't this work?

Image

Don't ask me, but it was never triggered in the first place... No console.log, you see?

Also, since you are obviously sending the message to the client already, why have the client watch the value and then this.setEditorValue? Isn't it much wiser to also run_javascript on the client to ensure setEditorValue gets ran?

evnchn avatar Apr 07 '25 17:04 evnchn

Ah, now I see! 🤯

So you are trying to set the codemirror value to something, say, "Y", right?

But, when you actually type into the textbox, it doesn't mutate the value of the element as seen in mounted_app.elements[4].props

Image

So, the first time it works, since the value went from "sub_page" to "Y". Second time onwards, to Vue, it went from "Y" to "Y", so it's NOT fine.

@falkoschindler see if this makes the situation clear enough for understanding this issue and how #4586 fixes things by forcing setEditorValue no matter what.


Regarding "we might as well need to handle language, theme and disable differently", I am not keen into it. This is because

  1. don't fix it if it ain't broken, and
  2. there is no way to change the language, theme, and disable without the involvement of NiceGUI (unlike value, which you can type into the codemirror, y'know), so there should not be such fall-out-of-sync issue, and
  3. there is no complex client-server sync operation like the application of ChangeSet, so even if it fell a bit out of sync, it'd be fine, at least not errors filling up the screen.

evnchn avatar Apr 07 '25 17:04 evnchn

It's been a while since the last update. Seeing that this is in the NiceGUI wishlist, I'd imagine it is semi-important, so I'd just like to see if there are any more additional clarifications that I shall make to push forward the issue and the fix in #4586

evnchn avatar Apr 18 '25 12:04 evnchn

@evnchn You're right, this issue is kind of important and I'd love to get it fixed. I had it on my list and looked into it from time to time. It's just that I really struggled to wrap my head around it. I've seen your PR, but something didn't feel right. I suspected there must be a better, simpler solution. Like with PR #4625 where we could remove the line in question in the end.

Now I gave it another shot and came up with a rather simple PR #4635 which could replace your PR #4586. What do you think?

falkoschindler avatar Apr 18 '25 16:04 falkoschindler

Congrats on the first "done" item in the NiceGUI Wishlist since probably a long time ago! 🎉

evnchn avatar Apr 18 '25 19:04 evnchn