slint icon indicating copy to clipboard operation
slint copied to clipboard

Feature proposal: Keep displaying preview if *.slint file is temporarily invalid

Open ubruhin opened this issue 1 year ago • 7 comments

The live preview is very handy to have it open while writing *.slint files, and the integrated VSCode preview is really awesome.

However, while typing, the *.slint file is temporarily invalid which causes the preview to disappear and only the parse error is shown. This has two drawbacks:

  1. While typing, you cannot look at the preview anymore to decide what to type next (e.g. what color, what border width, what icon size makes sense) because the preview is gone.
  2. After typing, at the moment the *.slint gets valid again, the preview appears again but you don't see the actual difference to the previous version. As a very trivial example, when changing background: red; to background: blue; the background does not switch directly from red to blue, but the preview completely disappears in between so you don't really see the effect of the code change.

I think it would be nice if the preview always stays at the last valid state, and is only updated when the *.slint file becomes valid again. This way, you will clearly see the effect of any code change.

Of course error messages shall still be displayed to make clear if the *.slint is invalid. Maybe as an overlay? Or with a symbol in the header bar, though this won't work when the bar is hidden :thinking: No clear opinion about this...

ubruhin avatar Sep 27 '24 15:09 ubruhin

This problem sometimes makes me work with the clipboard through cut and paste to be able to directly compare a difference. It would be nice if these cases became less noisy.

You could draw an analogy to windows under the Windows OS that aren't responding anymore (dialog on top doesn't appear right away). Their content is still displayed (cached by the OS). The DisableProcessWindowsGhosting() Windows API function docs explain the feature:

Disables the window ghosting feature for the calling GUI process. Window ghosting is a Windows Manager feature that lets the user minimize, move, or close the main window of an application that is not responding.

Enyium avatar Sep 27 '24 16:09 Enyium

So when should we refresh the window?

How should we report the errors in the preview?

Note that all the property information and everything else is wrong, so we need to block out those, even when we keep the preview visible.

hunger avatar Sep 30 '24 11:09 hunger

So when should we refresh the window?

Every time the code changes and is valid. I don't know the software architecture of the preview, but maybe something like this pseudo-code:

var rendered_ui;

on code_changed:
  var result = parse(new_code);
  if result.success:
    rendered_ui = result.ui;
  else:
    display_error(result.error);

How should we report the errors in the preview?

Currently I don't have a better idea so I think for now a simple overlay would be good enough:

image

If the toolbar is not shown (so the UI fills the window), the overlay would be within the displayed UI, but the "x" button could be used to hide the overlay (until a new error occurs).

Note that all the property information and everything else is wrong, so we need to block out those, even when we keep the preview visible.

I guess this "problem" exists already, not directly related to this feature request. The left and right docks are still there even if the *.slint is invalid:

image

The left dock changes to read-only, which makes sense. But the right dock somehow is still editable - probably it should switch to read-only too or completely disappear(?).

ubruhin avatar Sep 30 '24 12:09 ubruhin

How should we report the errors in the preview?

Currently I don't have a better idea so I think for now a simple overlay would be good enough:

Maybe a dialog-window-like, slightly translucent overlay that you can move and resize, and that word-wraps its text (there can be many errors at once). When you intermediately have no errors and then some again, the last position and size would be restored. This could be compared to a ghost window under Windows with an error dialog above it. You could always move it out of the way of what part of the UI you're currently working on.

the "x" button could be used to hide the overlay (until a new error occurs).

I'm not sure it should be closable. If it should, it would be vital to still have a visually prominent way of showing that there are errors and the current component display is a dead image. There should also be a way to get the closed overlay back.

Enyium avatar Sep 30 '24 13:09 Enyium

I like the overlay idea and keeping the functional state while typing.
I have a couple of thoughts.

  • agree that we should disable entry into the properties if they won't be updating
  • we could add a setting to either update live, as it is now, or only update page on saved.
  • the overlay window idea is nice but starts to get crowded - we could add something to the bottom status - error status with a count that then allows it to expand the bottom section and just make the overlay say something to the effect of "this preview is no longer live as there is an error" with a lint to expand the window. something like this

image

yes this might look familiar as it is the problem tab on vscode. I assume we are getting those from the language server. I do not know if it makes sense for it to pop open in the preview if you're on vscode... I think the 'not live' warning is enough and the link can take you back to the problems on vscode - however for those not using vscode, I don't know how clear that would be.

szecket avatar Sep 30 '24 14:09 szecket

agree that we should disable entry into the properties if they won't be updating

Note that properties should actually work even if there are compile error (well, depending how bad it is)
But if there was no parse error at least, the properties should still be accurate, regardless of the presence of the preview.

we could add a setting to either update live, as it is now, or only update page on saved.

Ideally this shouldn't be a setting but just a good default.

the overlay window idea is nice but starts to get crowded

Another idea I'd like to throw is that we could have some opacity or darker overlay of the previewed component when it is not "live" If we go with a bottom panel, i think it's important that it doesn't move" the preview. (it could itself have an opacity or would that be ugly?)

But overall i like the proposed change by @szecket Would be also nice if the error message can be selected (for https://github.com/slint-ui/slint/issues/6175)

ogoffart avatar Sep 30 '24 14:09 ogoffart

Note that properties should actually work even if there are compile error (well, depending how bad it is)

The property editing is based heavily on offsets in the file. So any added or removed characters before a property will break it. We use the file version to make sure the property edits we sent are applicable to the source code. Those will no longer match, so the Workspace Edit should get rejected.

Another idea I'd like to throw is that we could have some opacity or darker overlay of the previewed component when it is not "live"

I doubt our designers will like us messing with the colors ;-)

hunger avatar Oct 07 '24 09:10 hunger

Some additional suggestions on the behaviour of reload on save after Tobias, Auri and I discussed this a bit further in person.

In the following, suppose that reload on save is the active setting (or reload on type is disabled).

  1. While the user makes changes to any files that were used to produce the rendering, the live-preview is not updated. The user needs to save all files before the preview updates. Meanwhile, all editing functionality (library dnd, property editor) is disabled, until all files have been saved to "disk".
  2. When all files are saved, the preview is up-to-date, and editing functionality is enabled, the live-preview needs to acquire a lock on all files that are part of the preview the very moment the user makes a change. The lock should prevent the editor from permitting the user to make further changes to the files - the equivalent of disabling editing functionality in (1). The live-preview needs a "save" button & shortcut, and then the user saves the preview, the files need to be written to disk and the lock released.

tronical avatar Dec 05 '24 09:12 tronical

I think the OP's proposal as well as @szecket 's suggestion are implemented with #7015 , in what seems very satisfactory.

tronical avatar Dec 09 '24 15:12 tronical

If you all are also satisfied with the current solution (see nightly snapshot), then I suggest we close this issue and I'll open a new one for tracking the ability to make updates as you type a configurable feature.

tronical avatar Dec 09 '24 15:12 tronical

I am quite thrilled about this implementation after having tested it. Errors are still shown in the problem section in vscode. I wonder if that dialog can be popped open in a future update. Regardless, huge improvement.

szecket avatar Dec 09 '24 15:12 szecket

Awesome! Quickly tested it and I'm happy :smiley: Thanks for this great UX improvement.

ubruhin avatar Dec 09 '24 15:12 ubruhin

Closing and created #7040

tronical avatar Dec 09 '24 16:12 tronical