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

fix: correct Button elevation #2735

Merged
merged 3 commits into from
May 27, 2021
Merged

Conversation

tasugi
Copy link
Contributor

@tasugi tasugi commented May 19, 2021

Summary

It has a bug that Button elevation is still zero after mode is changed to contained from other modes.
This PR fixes this.

Test plan

  1. Render a Button with mode other than contained.
  2. Change the mode to contained mode dynamically(e.g. on button press).
  3. Check elevation

@callstack-bot
Copy link

callstack-bot commented May 19, 2021

Hey @tasugi, thank you for your pull request 🤗. The documentation from this branch can be viewed here.

@brunohkbx
Copy link
Collaborator

related to #2736? cc @lukewalczak

@lukewalczak
Copy link
Member

@tasugi could you please provide the snack to the issue which you're solving, where we can observe the bug?

@tasugi
Copy link
Contributor Author

tasugi commented May 25, 2021

@lukewalczak Sure! I created a snack. https://snack.expo.io/@tasugi/reactnativepaper-pr2735

If you press the button, you can change the mode of it contained. I think you can see the button is not elevated.( Element inspector may help you to check it)

Use useEffect

Co-authored-by: Luke Walczak <[email protected]>
@tasugi
Copy link
Contributor Author

tasugi commented May 27, 2021

@lukewalczak Thank you for suggestions!

@tasugi
Copy link
Contributor Author

tasugi commented May 27, 2021

Fixed an issue that caused the test to fail.

@lukewalczak
Copy link
Member

Thanks @tasugi!

@lukewalczak lukewalczak merged commit 0e024b8 into callstack:main May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants