Set height to 100% on popup version of elfinder

[jnoordsij]

WHY

BEFORE - What was wrong? What was happening before this PR?

On browse field, a popup containing the elFinder view is created. However the height of the appearing elFinder instance does not match the full height of the popup, causing weird whitespace to appear underneath it.

See also:

AFTER - What is happening after this PR?

The elFinder instance takes up the entire space.

HOW

How did you achieve that, in technical terms?

Pass a height 100% argument.

Is it a breaking change or non-breaking change?

Non-breaking

How can we test the before & after?

Use on the demo; example above is from https://demo.backpackforlaravel.com/admin/pet-shop/owner/5/edit.

[karandatwani92]

Hey @jnoordsij

Thanks for submitting the PR. Efforts from the Backpack community mean a lot to us. We will review it and come up with an update soon.

[karandatwani92]

Hey, @pxpm @tabacitu Just tested this and in my case, the popup has transparent-space(in all themes) instead of whitespace.

And suggested code change height: '100%', makes it cover that space.

Before:

After

[karandatwani92]

Also, I suggest adding theheight: '80%', in elfinder.blade.php for http://backpack-demo.test/admin/elfinder

[jnoordsij]

Hey, @pxpm @tabacitu Just tested this and in my case, the popup has transparent-space(in all themes) instead of whitespace.

And suggested code change height: '100%', makes it cover that space.

Before:

After

Hmmm, interesting! Still however you can notice that the popup itself is larger than the elFinder instance as the Laravel debugbar icon is now floating below.

I think adding 100% height should be done regardless of what the actual size of the instance should be; if you want a smaller popup, the browse CRUD field in the pro repo should be adjusted to create a smaller popup box (the popup there is currently set to 80% width/80% height).

[jnoordsij]

Also, I suggest adding theheight: '80%', in elfinder.blade.php for http://backpack-demo.test/admin/elfinder

Ah yes, I think I did this already in my own override. Not directly related to this issue, but if everyone agrees with enlarging that I'd be happy to commit it here.

[pxpm]

Hey @jnoordsij thanks for the PR.

Also thanks @karandatwani92 for the review.

I've noticed that @karandatwani92 is still using the old elfinder files as we changed the elfinder themes in 3.0.3 so it's possible that @jnoordsij is using them too when creating this PR.

I think this PR is not needed after https://github.com/Laravel-Backpack/FileManager/releases/tag/3.0.3

In that PR we added some css that will cover the scenario in this PR.

 #elfinder {
    height: 100% !important;
    width: 100% !important;
    top:0;
    left: 0;
}

Let me know what you think.

Cheers

[karandatwani92]

Yes, the 3.0.3 branch solved the issue. Closing it. Thanks for the PR @jnoordsij Thanks, @pxpm for taking care of things everywhere. 🙌🚀

[jnoordsij]

Ah yeah, that seems to do the same thing in another way. Hopefully can check it soon when updating my local views! The demo version linked above seems broken though for me atm...