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

Resize bug #100

Open
adriendst opened this issue Sep 2, 2024 · 34 comments
Open

Resize bug #100

adriendst opened this issue Sep 2, 2024 · 34 comments
Labels
bug Something isn't working

Comments

@adriendst
Copy link

Hello, I am using the library and I noticed some malfunction. I don't know if they are already known, so I wanted to share them to you.

The first one is :

bug.splitpane.1.mp4

In that case, when the top pane reach his minSize, he can still be minimized a little bit. Moreover, the when the top pane is smaller than his minSize, the bottom pane size reduce too. In the DOM we can see that the combined height of the three panes are over 100%.

The second one is :

bug.splitpane.2.mp4

In that case, when the first pane reach the end of the splitPanes component, both the first and last pane shrinks. The combined width of the three panes are less than 100%.

I was wondering if you could look at them ? Don't hesitate to ask me for more information :) (both the videos have been recorded from https://orefalo.github.io/svelte-splitpanes/ )

@orefalo orefalo added the bug Something isn't working label Sep 12, 2024
@orefalo
Copy link
Owner

orefalo commented Sep 12, 2024

Morning @adriendst,

Thank you so much for reporting the issues. I knew about the first one, not the second.

I honestly don't know when I will free up time to look into those.

@adriendst
Copy link
Author

Okay, let me know when you will get into it ! I did some adjustment in my own code to avoid those bugs for the moment

@hopperelec
Copy link

I've also experienced both of these issues while working on this PR in a separate repo. It is worth adding that there seems to be a discrepancy between what happens in Chromium- and Firefox-based browsers, which I and the other repo's maintainer have discussed more in the PR I linked.

@orefalo
Copy link
Owner

orefalo commented Sep 24, 2024

Identified the issue. it's a regression bug. may have to revert passive events.

anyways, stick to v0.8.2 for now

@orefalo
Copy link
Owner

orefalo commented Sep 24, 2024

found the bug. expect a release soon

@orefalo
Copy link
Owner

orefalo commented Sep 24, 2024

I believe this bug is resolved in version 8.0.6.
Please try

@hopperelec
Copy link

I'll try it, thanks! By the way, are you intentionally sticking with the 8 major versioning?

@hopperelec
Copy link

hopperelec commented Sep 24, 2024

The first issue seems to be fixed, but I'm still having an issue which most closely resembles the second issue. I'm not entirely sure what's causing it- as far as I understand, I'm not using any of the sizing parameters in any extraordinary ways- but if I set the initial size at all, then:

  • In Chromium-based browsers, the initial size for all panes will be 0
  • In Firefox-based browsers, the initial size is proportional to what was set, but slightly higher. For example, if I have a 1000px container and set the initial size to 40%, I expect the size to be 400px, but it is 441.33px

@hopperelec
Copy link

After a bit more testing, this seems to actually be an issue with reactivity. I'll try and fix the issue in the PR and hopefully I'll then have a better idea of exactly what it is.

@hopperelec
Copy link

hopperelec commented Sep 25, 2024

I don't recall getting this before, but I am now noticing these errors.
image
They only happen in Chromium and if the size has been set. If the issue occurs, they happen at the same time as the size changing to 0, but they also happen even if the issue doesn't occur. The issue seems to occur if the size starts as null/undefined then changes reactively to a numeric value. The error seems to indicate that the pane itself somehow becomes undefined. I have not yet found a fix for this.

@orefalo
Copy link
Owner

orefalo commented Sep 25, 2024

Good morning,

Regarding the version, I am sticking to 8.x.x. I made a mistake a few weeks ago, and it was too late to withdraw the package from npm. I plan to port to Svelte 5 as 9.x.x.

Regarding the exception, those are good tips. I will investigate. A REPL to reproduce the error would be helpful.

Best regards,

PS: size should not be null. set it to 0

@orefalo
Copy link
Owner

orefalo commented Sep 25, 2024

Seems to be going through this code.

  const reportGivenSizeChangeSafe = (size: number) => {
    // We put an extra check of `size != sz` here and not in the reactive statement, since we don't want a change
    //  of `sz` to trigger report.
    if (size != sz) {
      carefullClientCallbacks('reportGivenSizeChange')(size);
    }
  };
  $: {
    if (browser && size != null) {
      reportGivenSizeChangeSafe(size);
    }
  }

considering introducing a typeof value === 'number' but I need a REPL to test @hopperelec

@orefalo
Copy link
Owner

orefalo commented Oct 1, 2024

plz test the latest code changes in master - not release as a version yet.
if the bug comes from the component, it should fix it.
closing for now

@orefalo orefalo closed this as completed Oct 1, 2024
@hopperelec
Copy link

Oh sorry, been busy lately because I've just started uni. I'll try and test it ASAP!

@hopperelec
Copy link

hopperelec commented Oct 4, 2024

Ok, I tried creating a REPL for this using StackBlitz and now I'm even more confused about the issue.

I first tried with your changes in master (repl) where:

  • in the StackBlitz pane, it does the sizing similarly to Firefox, but the splitters don't show
  • in a new tab, all the panes would get 0 width but still show their content

But then, to make sure that these differences were because of your changes, I also made a REPL based on the exact code I've been testing with from the svelte-mosaic PR (repl) where:

  • in the StackBlitz pane, it is identical to how it looks in Firefox, even if I use it in Chromium
  • in a separate tab, the initial size (only of the pane with an explicit initial size set) is 0

So somehow, I am now able to reproduce 5 different types of behaviour:

  • If I'm running the npm version locally and visiting it using Chromium:
    the initial size for all panes is 0
  • If I'm running the npm version locally and visiting it using Firefox OR if I'm running the npm version on StackBlitz and viewing it from the editor pane in either browser:
    the initial size is proportional to what was set, but slightly higher
  • If I'm running the GitHub master version on StackBlitz and viewing it from the editor pane (in either browser):
    the initial size is proportional to what was set, but slightly higher, and the splitters don't show
  • If I'm running the GitHub master version on StackBlitz and viewing it from a separate tab (only on Chromium):
    all the panes would get 0 width but still show their content
  • If I'm running the npm version on StackBlitz and viewing it from a separate tab (only on Chromium):
    the initial size (only of the pane with an explicit initial size set) is 0

Leodog896 has the same issues as me when running it locally, so I'm confident that this is not just something weird with my specific setup. My first guess as to what could cause the difference between running it locally and on StackBlitz was node versions, since I use Node 20 locally and StackBlitz only supports Node 18, however the difference between viewing it in-editor and in a separate tab makes me think it's more likely to be related to StackBlitz's WebContainers. More local testing could be done to narrow down what is causing these differences (running GitHub master version locally, running both versions locally using Node 18, setting up WebContainers locally) but, with there being so many differences to test now, I'm not really sure where to start, and I'm hoping someone else will have a better idea from the information I've already found.

@orefalo orefalo reopened this Oct 5, 2024
@orefalo
Copy link
Owner

orefalo commented Oct 5, 2024

First and foremost, thank you for setting up the test.s

  • Your first RPL confused me; it took a while to realize the code was duplicated—I suspect the invisible splitters are coming from a bundling problem. Don't want to troubleshoot further
  • The second RPL is intriguing. I noticed different sizing (and dragging) behaviors if I switch from versions 0.8.2 to 8.0.6.

I will need a bit more time to identify the root cause, and I plan to release the changes in master as version 8.0.7 for this purpose.

Will keep you posted.

@orefalo
Copy link
Owner

orefalo commented Oct 5, 2024

Released 8.0.7 - didn't make any significative changes other than releasing the code from master.

Tried it against your second REPL.
seems to be working fine, I can't reproduce the 0 size issue on chrome or firefox.

However, there is an error on the console: TypeError: Failed to construct 'URL': Invalid URL, which might be coming from code.

@hopperelec
Copy link

hopperelec commented Oct 5, 2024

it took a while to realize the code was duplicated

Ye, it was a bit of a weird setup, but I wasn't sure of a better way to do it in StackBlitz. Although I haven't used StackBlitz very much, so it is very possible I'm missing something here. I also did have to strip down the repo a bit since some of the linting packages required Node 20 which StackBlitz doesn't support and I didn't want to figure out which specific ones. Do let me know if you know a better way to go about testing unpublished changes!

Tried it against your second REPL.
seems to be working fine, I can't reproduce the 0 size issue on chrome or firefox.

Just tried it myself and I'm still getting the 0 size issue in Chromium if I open it in a separate tab. Just to confirm, did you try by opening it in a new tab?

However, there is an error on the console: TypeError: Failed to construct 'URL': Invalid URL, which might be coming from code.

I'm not getting any errors in the server console, and in the browser console I only get the same errors I showed a couple weeks ago.
image

@orefalo
Copy link
Owner

orefalo commented Oct 5, 2024

I think I understood what you meant by new tab... you mean maximizing starblitz internal browser, which opens a new tab
CleanShot 2024-10-05 at 18 15 23

Now I do see a funny effect, looks like the rendering flickers and is overlayed by a blank area,
to be honest, I doubt this is related to splitpane as I can see an IFRAME overlayed in the html rendering
CleanShot 2024-10-05 at 18 17 24

I don't see any of the errors you are pointing, this is what i get...
CleanShot 2024-10-05 at 18 20 49

@hopperelec
Copy link

I don't see any of the errors you are pointing, this is what i get...

Oh, those are more likely to be related to StackBlitz itself; check the browser console with it open in a new tab and you should see the console actually for the svelte-mosaic example page

@hopperelec
Copy link

to be honest, I doubt this is related to splitpane as I can see an IFRAME overlayed in the html rendering

Not sure where that IFRAME is coming from, but it is completely blank and deleting it does not make a difference, so I don't think this is the issue

@orefalo
Copy link
Owner

orefalo commented Oct 5, 2024

ok.. I see it now. weird.

Not sure where that IFRAME is coming from, but it is completely blank and deleting it does not make a difference, so I don't think this is the issue

No idea what Starblitz is doing,
Do you know how to export the project? I want to test locally.. I have no trust in Starblitz

@hopperelec
Copy link

Do you know how to export the project? I want to test locally

StackBlitz has a "Download as ZIP" button at the top of the explorer pane, but the easier way to test locally would be to just clone the svelte-mosaic PR https://github.com/hopperelec/svelte-mosaic/tree/splitpanes

@orefalo
Copy link
Owner

orefalo commented Oct 5, 2024

the git repo above is the bundled version, wont' work

trying to download,

@hopperelec
Copy link

What do you mean by "the bundled version"?

@orefalo
Copy link
Owner

orefalo commented Oct 5, 2024

bundled - you are not using splipanes as a dependency. i don't want to debug code bundling.

I can't download this damn project, I hate starblitz

@hopperelec
Copy link

you are not using splipanes as a dependency

I am? It is using version 0.8.2 right now, though. The only place I used it bundled was in the first StackBlitz REPL; the git repo I linked to does not do that

@orefalo
Copy link
Owner

orefalo commented Oct 5, 2024

this is your package.json - there is no refs to splitpanes,

CleanShot 2024-10-05 at 18 48 25

How can I get the 2nd REPL from starblitz as a zip file?

@hopperelec
Copy link

hopperelec commented Oct 5, 2024

this is your package.json - there is no refs to splitpanes,

It looks like you cloned the master branch, not the splitpanes branch I linked to. svelte-mosaic used to handle the panes on its own, but the PR I created is to migrate it to using splitpanes. The PR hasn't been merged yet because of the issues, so it still lives on a separate branch.

How can I get the 2nd REPL from starblitz as a zip file?

image
image

@orefalo
Copy link
Owner

orefalo commented Oct 5, 2024

my bad, got it now.
interesting... looking into it

@orefalo
Copy link
Owner

orefalo commented Oct 5, 2024

The issue has been identified, and version 8.0.8 is being released.

Thank you for highlighting this issue. The root cause remains unclear, but the problem has been resolved.

Please try, confirm and close this ticket.

@hopperelec
Copy link

hopperelec commented Oct 5, 2024

Thanks so much for looking into the issue! Sadly, it still doesn't seem to be fully working for me- it is very similar to 8.0.6

However, in Firefox, the initial size now gets precise if the container is resized, which wasn't the case before.

To re-iterate for clarity:

  • In Chromium, the initial size (only of the pane with an explicit initial size set) is 0, but clicking on the splitter causes it to jump to the minimum size.
  • In Firefox, the initial size is proportional to what was set, but slightly higher unless the container is resized. Resizing the container after everything has rendered causes the exact expected behaviour!

I should also add for full clarity that, in both browsers, it shows as 50% width very briefly before going to the initial size I mentioned. This has been the case the whole time and I assume is just part of the rendering process. I mention this since you mentioned the rendering "flickering" earlier.

@orefalo
Copy link
Owner

orefalo commented Oct 6, 2024

Morning,
Correct I can reproduce what you are describing above.... but I thought it was per design ;-/

I changed your code as such -

	$: { initial = containerSizePx && getInitial()

		console.log("initial="+initial)
	};

CleanShot 2024-10-06 at 10 05 19

my point is, there is no way the component can render proper if the size is undefined. am I missing something?

@hopperelec
Copy link

Undefined should be the default value, meaning it should invoke the default behaviour, no? I have used undefined for panes where a specific size is not provided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants