Skip to content
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

Bug: Border does not render if background is transparent #145

Closed
bryphe opened this issue Dec 22, 2018 · 5 comments
Closed

Bug: Border does not render if background is transparent #145

bryphe opened this issue Dec 22, 2018 · 5 comments
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@bryphe
Copy link
Member

bryphe commented Dec 22, 2018

Issue: If a border is specified, but no background, the border does not render.

For example, for this component:

module Test = (
  val component((render, ~children, ()) =>
        render(
          () => {
            let borderStyle =
              Style.make(
                ~border=Style.Border.make(~width=10, ~color=Colors.white, ()),
                (),
              );

            let innerStyle =
                Style.make(
                    ~backgroundColor=Colors.red,
                    ~width=100,
                    ~height=100,
                    (),
                );

              <view style=borderStyle>
                  <view style=innerStyle />
              </view>
          },
          ~children,
        )
      )
);

I would expect to see this:
image

But get:
image

I actually hit this in #138 , which is why I added a pretty transparent background here:
https://github.com/bryphe/revery/blob/d05c343e67c90b81fdc19da74fbf59b5e6a87536/examples/Bin.re#L113

The issue seems to be this check: https://github.com/bryphe/revery/blob/d05c343e67c90b81fdc19da74fbf59b5e6a87536/src/UI/ViewNode.re#L241

@bryphe bryphe added bug Something isn't working help wanted Extra attention is needed good first issue Good for newcomers labels Dec 22, 2018
@OhadRau
Copy link
Collaborator

OhadRau commented Dec 22, 2018

Whoops, I think in my head I was looking at that c.a as style.opacity when I went back to check that code. I'll fix this real quick

@bryphe
Copy link
Member Author

bryphe commented Dec 24, 2018

Fixed by #147 - thanks @OhadRau !

@bryphe bryphe closed this as completed Dec 24, 2018
@OhadRau
Copy link
Collaborator

OhadRau commented Dec 26, 2018

I think this snuck back in when the box-shadows were merged...

@bryphe
Copy link
Member Author

bryphe commented Dec 26, 2018

Ah, bummer - thanks for catching it @OhadRau . I'll reopen to track.

Also trying to think about ways to catch these sorts of regressions via CI - put some thoughts in #156

@OhadRau
Copy link
Collaborator

OhadRau commented Dec 26, 2018

@bryphe @Akin909 Fixing this right now. Looks like shadows are currently going to be rendered only if the node's alpha > 0 (so essentially the same as this bug). Is this the desired behavior or should I make that happen regardless of the node's alpha?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants