IDEMSInternational / open-app-builder

PLH App Frontend
GNU General Public License v3.0
6 stars 25 forks source link

fix: button child layout positioning #2289

Closed chrismclarke closed 5 months ago

chrismclarke commented 5 months ago

PR Checklist

Description

Inconsistencies have been noticed to the way different browsers handle toggle element nested inside button for comp_button template. This is likely due to different ways the position: absolute property of the rendered children span is interpreted (this should be nearest element that has relative positioning, it appears chrome stops if it doesn't find an element when reaching shadow-dom parent component whilst webkit continues further up dom root).

This PR explicitly sets relative positioning on the button container element to ensure children are rendered with correct positioning

Review Notes

As the bug has only been noticed on ios/safari browser, testing on windows should be done via the playwright test framework, accessing the underlying webkit browser

npx playwright install
npx playwright wk http://localhost:4200/template/comp_button

Git Issues

Closes #2262

Screenshots/Videos

Before - Portrait card toggle positioned in absolute top-right of screen image

After - Portrait card toggle positioned correctly image

chrismclarke commented 5 months ago

Great, I'm pleased you managed to get to the bottom of this. And good to have a method of testing on a webkit browser, although this was the only one of the three bugs referenced in #2288 that I could recreate on desktop Safari, the other two were only present on iOS safari/native as far as I can tell.

Thanks, yeah I think actually quite good to try and decouple these issues instead of fixing altogether as I have a feeling there's a few issues going on under the hood (as evidenced by the fact some appear in ios and others don't). From what we've unearthed so far I'm guessing a mix of browser CSS spec interpretation/ambiguous component css, browser bugs and other hardware issues (e.g. hardware acceleration).

So after this I'd probably recommend checking similar components that have absolute positioning layout (ideally all elements that refer to position:absolute should also have a parent component with position:relative to position in relation to. Unfortuantely this appears to be quite a lot so might just be something we pick up case-by-case as/when things come up image

Otherwise it might be worth considering updating our visual test system to include other browsers (worth a follow-up issue/discussion?) - This wouldn't be totally trivial as I don't think pupetteer headless browser runner supports webkit, so would mean migrating code to use playwright instead.