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(drawer): fix drawer when changing drawertype (ex: front -> permanent) #10304

Merged
merged 2 commits into from
Feb 1, 2022

Conversation

SherifMega
Copy link
Contributor

regarding to
#10210

and after latest updates for
5246574

…e permanent to prevent white stretched block
@github-actions
Copy link

Hey shrief1234567899! Thanks for opening your first pull request in this repo. If you haven't already, make sure to read our contribution guidelines.

@satya164
Copy link
Member

What exactly is this fixing?

@netlify
Copy link

netlify bot commented Jan 28, 2022

✔️ Deploy Preview for react-navigation-example ready!

🔨 Explore the source changes: 338f7c9

🔍 Inspect the deploy log: https://app.netlify.com/sites/react-navigation-example/deploys/61f46da0fae4ab0007874d08

😎 Browse the preview: https://deploy-preview-10304--react-navigation-example.netlify.app/

@sherif-cyber
Copy link

sherif-cyber commented Jan 28, 2022

What exactly is this fixing?

fixing white block in screen when

  • open drawer as front drawerType
  • switching from portrait to landscape mode
  • change drawerType to be Permanent instead of front

if you do that you will notice white block beside drawer section
as mentioned here in the video
#10210

@satya164
Copy link
Member

I already made a fix in 5246574, what does this do additionally?

@sherif-cyber
Copy link

I already made a fix in 5246574, what does this do additionally?

i just changed undefined transform to be zero in translateX

because without this change, we still face an issue (wrong position for drawer in middle of screen and white space beside it) when changing drawerType within Runtime from front to be permanent as appeared in video.

@satya164
Copy link
Member

because without this change, we still face an issue (wrong position for drawer in middle of screen and white space beside it) when changing drawerType within Runtime from front to be permanent as appeared in video.

Can you open a new bug report for the issue you encountered?

i just changed undefined transform to be zero in translateX

I can see that in the commit, but the mentioned commit (and the code comments) already say why I used undefined and not a transform. So just changing it to transform is not a fix.

@sherif-cyber
Copy link

because without this change, we still face an issue (wrong position for drawer in middle of screen and white space beside it) when changing drawerType within Runtime from front to be permanent as appeared in video.

Can you open a new bug report for the issue you encountered?

i just changed undefined transform to be zero in translateX

I can see that in the commit, but the mentioned commit (and the code comments) already say why I used undefined and not a transform. So just changing it to transform is not a fix.

umm, i removed using translateX and now i am using empty array instead of it, and it's also working fine for me.

when we use undefined it cause a design bug (same bug in mentioned video)

@codecov-commenter
Copy link

Codecov Report

Merging #10304 (338f7c9) into main (28eb82f) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #10304   +/-   ##
=======================================
  Coverage   74.28%   74.28%           
=======================================
  Files         160      160           
  Lines        4881     4881           
  Branches     1850     1850           
=======================================
  Hits         3626     3626           
  Misses       1220     1220           
  Partials       35       35           
Impacted Files Coverage Δ
packages/drawer/src/views/legacy/Drawer.tsx 59.77% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28eb82f...338f7c9. Read the comment docs.

@sherif-cyber
Copy link

because without this change, we still face an issue (wrong position for drawer in middle of screen and white space beside it) when changing drawerType within Runtime from front to be permanent as appeared in video.

Can you open a new bug report for the issue you encountered?

i just changed undefined transform to be zero in translateX

I can see that in the commit, but the mentioned commit (and the code comments) already say why I used undefined and not a transform. So just changing it to transform is not a fix.

umm, i removed using translateX and now i am using empty array instead of it, and it's also working fine for me.

when we use undefined it cause a design bug (same bug in mentioned video)

can we merge it ? @satya164

@adamhari
Copy link

adamhari commented Feb 1, 2022

I've been locked to 6.1.4 because the changes in 6.1.5 broke the drawer like this when switching the drawerType. This PR fixes it for me.

Copy link
Member

@satya164 satya164 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can confirm [] fixes the issue. thank you

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.

None yet

5 participants