search

NoDivide / astrum

A lightweight pattern library designed to be included with any web project.
http://astrum.nodividestudio.com
1.54k stars 98 forks source link

Implemented overflow fix for preloaded container #153

Closed craigpryde closed 6 years ago

craigpryde commented 6 years ago

What does this PR cover?

Implemented a css update to fix overflow issue detailed in: #152

How can this be tested?

Compile and run

Reviewers

Review 1

By adding a +1 you are confirming you have...

Andy-set-studio commented 6 years ago

Hey @craigpryde! Thanks so much for finding this and putting in a PR. We really appreciate it.

It looks like a fair enough fix, but how come you've set .ndpl-preloaded to display: table and not left/set it as display: block?

The overflow rule should work because of the absolute positioning that's been set in my opinion.

There's a good chance that I'm being ignorant, but it would be great to know :)

craigpryde commented 6 years ago

Hey @hankchizljaw,

Thanks for the response. I did try display:block; on the preloaded container but this appeared to have no effect on the component and it still rendered in the same way. image

I suspect this is due to the component not having a display property set which would mean it would be set to display:block; on chrome by default. image

It was after trying display block that I changed the display to table to force the browser to render the container differently. I'm sure the display block; solution would work under most circumstances and would be my first pick also but from my experience, it can cause rendering issues like the above when used with a lot of flexbox based child elements causing the overflow.

I did, however, dive deeper into the issue and found adding width:0px; onto the element with display block also solved the overlapping issue but this resulted in the components displaying a "This component is hidden at this resolution." message that must have been caused by the JS when the hidden content is preloaded. image

Because of that, I chose to use the display: table; property as this seemed to rectify the bug and from other projects, I have found this to be the most reliable.

Would be great to know your thoughts :)

Andy-set-studio commented 6 years ago

Hey @craigpryde, so sorry about the delay!

I think that's all fair enough and well-balanced.

Let's merge it in 🚀

gaetanmarsault commented 6 years ago

Hi everbody, the solution (display: table on .ndpl-preloaded) seems not working in IE 11... have you got any idea ? Thx