Extra wrapper `<div>` makes `height: 100%` not work

[jaens]

Are you certain it's a bug?

• Yes, it looks like a bug

Are you sure this is not an issue in noVNC?

• It is not a noVNC issue

Is there an existing issue for this?

• I have searched existing issues, it hasn't been reported yet

Issue description

The VncScreen component adds an extra unstyleable wrapper <div> around the main noVNC <div>, which means setting <VncScreen style={{ height: '100%' }} ...> does nothing, since the height of the wrapper <div> will default to 0 (ie. the component will just turn invisible) due to the CSS box model.

(height: 100% is particularly useful in conjunction with scaleViewport and flexbox to eg. render an almost fullscreen VNC but with a variable-sized header/toolbar before it)

Right now, there's no good way to make this work nicely (without CSS children selector > div hacks and even more wrapper divs anyway)

Location of issue:

react-vnc/src/lib/VncScreen.tsx

Lines 344 to 354 in 66221f0

	return (
	<div>
	<div
	style={style}
	className={className}
	ref={screen}
	onMouseEnter={handleMouseEnter}
	onMouseLeave={handleMouseLeave}
	/>
	{loading && (loadingUI ?? <div className="text-white loading">Loading...</div>)}
	</div>

I propose that the <div> should be a fragment instead to make it work nicely with all CSS layouts (ie. return <> <div>...</div> { ... } </>)

(this issue was introduced in d716043)

[roerohan]

Hey @jaens, thanks for raising an issue!

You're right, the outer <div> causes this issue. I don't remember why I changed it from using a <></> fragment wrapper to using a div, but this was a recent change. I think reverting to <></> would be the simplest fix, although I was wondering if the Loading... bit is useful at all. The only reason it was there was to indicate that the RFB object was in fact created in case someone runs into an issue. Although, I don't think it's really useful, it's quite easy to implement it outside the library. What do you think?

[jmgrosen]

That fix sounds good to me! I would love if this were fixed soon.

[roerohan]

Hey, just made a PR #66 for this issue, I'll merge it now to close this issue.

[github-actions]

🎉 This issue has been resolved in version 3.0.8 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀