sxyazi / yazi

💥 Blazing fast terminal file manager written in Rust, based on async I/O.
https://yazi-rs.github.io
MIT License
14.58k stars 330 forks source link

Input Widgets Don't Seem to Be Correctly Rendered Over Preview #347

Closed edgeyyzhang closed 9 months ago

edgeyyzhang commented 10 months ago

What system are you running Yazi on?

macOS

What terminal are you running Yazi in?

iTerm2

Yazi version

0.1.5

Did you try the latest main branch to see if the problem has already been fixed?

Tried, but the problem is still present

Describe the bug

Part of the preview is rendered over the input widget, while part of the input widget doesn't go away after cancelling the input.

https://github.com/sxyazi/yazi/assets/109451175/99a6d625-7c06-4058-92d6-c7c9d7c769af

https://github.com/sxyazi/yazi/assets/109451175/53c6a43a-3418-41e8-803e-cc04cadd6201

Expected Behavior

  1. The input widget should be on top of the preview.
  2. The preview should be restored after the input widget is canceled/submitted.

To Reproduce

Open Yazi in terminal windows with less columns, so that the input widget overlaps the preview. Open and cancel the input widget.

Configuration

No response

Anything else?

No response

sxyazi commented 10 months ago

This is a known bug, and I haven't found the time to fix it yet, in case someone wants to create a PR for it. Here are some ideas:

  1. When the Input component is visible=true, get the position of the Input component: https://github.com/sxyazi/yazi/blob/7c49d9c57a8eb5f1c3263037b9424b55f2306735/yazi-fm/src/input/input.rs#L19-L20

  2. Implement an image_clear method for the Adaptor, which takes the position of the Input component and clears the image in that area, finely like the existing image_hide:

    https://github.com/sxyazi/yazi/blob/7c49d9c57a8eb5f1c3263037b9424b55f2306735/yazi-adaptor/src/adaptor.rs#L182-L195

Yazi supports 3 image protocols: Kitty, iTerm2, and Sixel, there's no need to implement all of them. Any constructive PRs are highly welcome!

edgeyyzhang commented 10 months ago

Thanks for your suggestion! I tried a bit but failed for now. It seems we need to rewrite the Iterm2::encode function to get rid of the cells in the Input component. Using crossterm::terminal::clear may not work because they don't provide function to reset a single cells (I'm not 100% sure though).

Another workaround is to draw the cells within the Input component. However, the rendering for this area is handled by paragraph from ratatui::widgets:: now. Maybe we can implement our own render function. This seems to be a easier way for our goal, and it doesn't involve passing through the Input information from Apps all the way down to Adaptors. Personally, I would prefer this one.

Will give it another try when I have time!

sxyazi commented 10 months ago

For the iTerm2/Sixel protocol, it doesn't have a true concept of "clear". It just:

  1. Simply move the cursor to the corresponding cell, by Term::move_to()
  2. Using a blank character (" ") to fill it: stdout.write_all(" ".repeat(width))

It's exactly the same as what image_hide does now, except that it doesn't currently clear the input area when it's visible:

https://github.com/sxyazi/yazi/blob/a0ba8537188cd5c29593b1c53a3569ac7f1a0bd7/yazi-adaptor/src/sixel.rs#L22-L32

github-actions[bot] commented 8 months ago

I'm going to lock this issue because it has been closed for 30 days. ⏳ This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.