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

Creating Box Shadows before first render #7

Closed
alexco2 opened this issue May 24, 2021 · 8 comments
Closed

Creating Box Shadows before first render #7

alexco2 opened this issue May 24, 2021 · 8 comments

Comments

@alexco2
Copy link

alexco2 commented May 24, 2021

Hello,
I tried react-native-neomorph-shadows, but the problem with the missing shadow until the first render is more or less a dealbreaker for me. Also, calling the onLayout method multiple times when navigating is also kind of heavy on the js thread, which causes dropped frames. You wrote you might have a solution to this? I´m really interesed to hear what your thoughts are. Anyway, thank you for your contribution. React native really needs better shadow support for android.

@SrBrahma
Copy link
Owner

About onLayout: the issue is if the view changes its size. Maybe I could add a prop like onlyOnceOnLayout, and it would simply use a useState to only allow the onLayout prop if the state isn't set yet . Can you manually test if it would really increase your performance? Also, do you know about useMemo? Maybe it can also increase the performance.

About the same render shadow, this was my test of it when I was developing this package, as seen in the end of the index.tsx:

      {/* Experimental: This below allows overflowing the children size.
        However, can't find a way to set the viewBox with this approach, so the gaps would probably happen.
        Would it sucessfuly use the style width and height as viewBox without gap?
        Maybe could use this on first render, then use the childLayout to set the viewBox sizing. The gaps would
        only happen on first render, but being it barely noticeable wouldn't be an issue. Better than no shadow on 1st render.
      */}
      <Svg style={{ position: 'absolute', width: '110%', height: '100%', left: -10 }}>
        <Rect height={10} width={'100%'} fill={'#0f0'} y={'50%'}/>
      </Svg>

I don't remember exactly the reasons of the magic values there, but I think the 110% was an increased View size to allow the shadow size in the SVG bounds, and the -10 was the shadow distance.

That may be problematic: e.g: Your view is 100px width, and you want a giant 50px shadow. The 110% would only allow a 10px shadow for one side. So, either the % would be great by default (potentially resource consuming, as if I understand the functioning of the SVG, it will render a big image), or maybe manually changeable by the dev. Like, you know 140% would always fit for your case.

IIRC, it worked but between each shadow piece (4 sides and 4 corners), there might be a pixel wide gap or an overlap of the shadows. As after the first render it would use the onLayout, only trained and quick eyes would notice that.

I probably won't work on that right now, as I am engaged in another project and have some other requested stuff on another ones in my queue. However, it shouldn't be too difficult for you to find good solutions. I would, and surely the other devs, appreciate a lot a good PR :-)

@SrBrahma
Copy link
Owner

Implemented in #11, version 3.0.0! Please, read the new README, try it out and report any bug!

@alexco2
Copy link
Author

alexco2 commented Jul 17, 2021

@SrBrahma wow, amazing. Thank you so much for your hard work. I will report any bugs as soon as I am able to try it out (which is definitely on my to do list).

@alexco2
Copy link
Author

alexco2 commented Aug 16, 2021

@SrBrahma
I now tested the library thoroughly and implemented with it neumorphism in my app. The first render issue from other libraries are not visible. So far it works really well, thank you! 😄
One issue I came across was in rare cases regarding the dimensions of a component which is wrapped with <Shadow>
I made a snack showing the problem:
https://snack.expo.dev/@expoco2/shadow-2
The first component shows the width I would expect the component to have. The second component shows the issue I came across. The width "shrinks". Did I perhaps miss something in the documentation?
Also, is it possible to pass a width but not a height to <Shadow>? Would this increases performance for cases where just the height is dynamic?

@SrBrahma
Copy link
Owner

SrBrahma commented Aug 16, 2021

@alexco2 , you may pass viewStyle={{alignSelf: 'stretch'}} for the current version (v3). Tested here and it worked. Thanks for the bug report! It's really important.

In the Shadow code, I am using alignSelf: 'flex-start' to fix another bug.

I will reopen this until I fix your case. Maybe changing that 'flex-start' to 'stretch' would also fix that other bug.

For the width but not height: no, that wouldn't increase the performance. It would still trigger a second render to apply the received onLayout size.

@SrBrahma SrBrahma reopened this Aug 16, 2021
@SrBrahma
Copy link
Owner

SrBrahma commented Aug 16, 2021

No, setting the default as 'stretch' also causes the bug.

This is the bug in question:

image

(having the shadow child component a fixed size)

This happens when you have:

<View>
  <Shadow ...>
    {component}
  </Shadow>
  <aLongSiblingComponent/>
</View

I will leave a tip in the README about this stretch workaround.

Edit: For future references, in your case, alignSelf: undefined would do the same. Maybe I can create a new prop, stretch or grow to have all the available size or not. I don't know what default value would be the best. The best case would be if that wasn't necessary at all. It could check if the child has a fixed size, and if so, use the flex-start. Else, undefined to use all available space. As I am without time right now for further research, will leave it to be done.

Suggestions are welcome.

@alexco2
Copy link
Author

alexco2 commented Aug 17, 2021

@SrBrahma
Really interesting, thanks for the clarification.
My case was simply solved by passing a width to the child view (in my case 0.9* Screenwidth).
If I come up with a clever way to handle this issue, I will let you know :)

@SrBrahma
Copy link
Owner

I am closing this as I have added this issue and its 'stretch' workaround to the FAQ and I have no idea to better handle it right now.

Once again, thanks for the bug report and feel free to comment here! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants