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

Fixed issue with set_target_size passing the wrong value to plugins #1657

Merged
merged 2 commits into from
Nov 28, 2023

Conversation

drmargarido
Copy link
Contributor

This happens for plugins that are split on the right and activated from the settings UI.

Fixes issue drmargarido/TodoTreeView#9 reported on the TodoTreeView plugin.

…hat are split on the right and activated from the settings UI.
@drmargarido
Copy link
Contributor Author

drmargarido commented Oct 28, 2023

The specific issue is that the value variable of the resize_child_node function goes from 0 to the screen size, but the node.size may be smaller so the value being sent (node.size[axis] - value) for the resize may be negative.

@Guldoman
Copy link
Member

What are the steps to reproduce the issue?

@drmargarido
Copy link
Contributor Author

drmargarido commented Oct 29, 2023

Steps to reproduce:

  1. Add the todotreeview-xl to the plugins folder
  2. Start LiteXL
  3. Disable todotreeview-xl in the settings UI
  4. Close LiteXL
  5. Open LiteXL
  6. Enable the todotreeview-xl plugin in the settings UI
  7. Try to resize the todotreeview-xl panel

Current behaviour: The panel goes to a really weird position or disappears
Expected behaviour: The panel resizes normally

@Guldoman
Copy link
Member

I think this should be done for this too:

local accept_resize = node.a:resize(axis, value)

So I guess value should either be translated at the start of resize_child_node or by the caller.

This was already done in #1516, but if you do the same fix for RootView:on_touch_moved, I'd just merge this instead.
In the end the issue I raised in that PR is still valid, but at least we can merge this for now.

Btw as a consequence of how you're doing the initial split in todotreeview-xl, if you already have multiple splits, and settings is not the rightmost one, the panel is opened at its right, leading to weird splits like:
image

We have a plan for a better way to deal with "panels", but it'll have to wait for now.

@drmargarido
Copy link
Contributor Author

Updated with the changes to on_touch_moved. Any recommended way to get the initial split to work as expected in the plugin? Or it's a thing to do only after your "panels" plan is ready?

@Guldoman
Copy link
Member

Any recommended way to get the initial split to work as expected in the plugin? Or it's a thing to do only after your "panels" plan is ready?

Sadly I don't think there's a good way with the current system.

jgmdev pushed a commit to pragtical/pragtical that referenced this pull request Oct 29, 2023
@takase1121 takase1121 merged commit 55a19cd into lite-xl:master Nov 28, 2023
1 check passed
takase1121 pushed a commit to takase1121/lite-xl that referenced this pull request Nov 30, 2023
…ite-xl#1657)

* Fixed issue with set_target_size passing the wrong value to plugins that are split on the right and activated from the settings UI.

* Added position awareness for the all resize_child_node calls.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants