-
Notifications
You must be signed in to change notification settings - Fork 29
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
Background as drawable #550
base: main
Are you sure you want to change the base?
Background as drawable #550
Conversation
439e1d5
to
4eeb5b7
Compare
This PR is based on this issue: #496 That issue gives example code that should change behaviour: Shoes.app(width: 600, height: 50) do
para "This is hidden under the background"
background yellow
para "This shows through on top"
end When I run this with the code in the branch, I see both pieces of text. So that's not perfect - the background drawable should cover the first piece of text. If I run with --debug, this is the HTML that gets sent (some backslashes added): <div id=\\\"2\\\" style=\\\"display:flex;flex-direction:row;flex-wrap:wrap;align-content:flex-start;justify-content:flex-start;align-items:flex-start;width:100%;height:100%\\\"><div style=\\\"height:100%;width:100%;position:relative\\\"><p id=\\\"3\\\" style=\\\"font-size:12px\\\">This is hidden under the background</p><div id=\\\"4\\\" style=\\\"height:inherit;width:inherit;position:absolute;top:0;left:0;z-index:-99;box-sizing:border-box;background:rgba(255, 255, 0, 255);border-color:rgba(255, 255, 0, 255);border-width:1px;border-radius:px\\\"></div><p id=\\\"5\\\" style=\\\"font-size:12px\\\">This shows through on top</p><div id=\\\"root-fonts\\\"></div><div id=\\\"root-alerts\\\"> </div></div></div> That's nearly right. But the z-index of -99 means that it is not covering the first piece of text, though it should. I think if that z index is removed it'll do what it should for that example. I'm also seeing some oddity with the example in the branch because the background has been modified to add a curve. So it's harder for me to tell if it's doing the right thing. |
The change we were looking at when pairing:
I think we need to use CSS isolation to do our own stacking context for everything since we're managing it all anyway. |
Still seeing a lot of test failures, presumably because of the "isolate" thing we talked about. |
yes. I have regenerated the fixtures as discussed. |
correct me if i am wrong i dont remember this exactly but - we need to generate fixtures only if we change current examples or add examples right? for test failures we have to change expected outputs manually ? |
@Pavan-Nambi Exactly right, yeah. In this case he's adding a CSS attribute to a lot of tags, so both those things will need to change. |
Description
Image(if needed, helps for a faster review)
Checklist