[iOS] Wrong bottom sheet opacity on onboarding and in the activity center #19319
Conversation
Jenkins BuildsClick to see older builds (22)
|
| @@ -119,7 +119,7 @@ | |||
| [blur/ios-view | |||
| {:style style/shell-bg | |||
| :blur-radius (or blur-radius 20) | |||
| :blur-type :transparent | |||
| :blur-type :dark | |||
There was a problem hiding this comment.
dunno how it's related, because it's only for shell? true ? but in the description not shell screens. cc @Parveshdhull
There was a problem hiding this comment.
hi @flexsurfer, for onboarding screen we are passing :shell? true
There was a problem hiding this comment.
@alwx I am not sure if using :blur-type as :dark is right solution.
:dark, :light also creates dark overlay, and if you check screenshot added in description. It is blackish and don't match figma.
There was a problem hiding this comment.
Fixing blur will be a little hard.
Quick fix
- As a background, we are passing
colors/white-opa-5, try usingcolors/white-opa-10etc. to check if it matches the design - Also you can play around with blur radius/amount
Another fix (probably not priority)
For exactly matching figma design, we will need to find exact combination of blur params and background color. Values which are used in figma can't be used directly, as :transparent applies some staturation.
More research: #14903
Related Discussion: Kureev/react-native-blur#513
We fixed blur issue for bottom tabs, as you can check in #14903 (comment), but it took lots of time.
There was a problem hiding this comment.
@Parveshdhull I played with values, nothing else gets even close to the designs except changing the blur amount. Changing background color makes it lighter or darker but doesn't change the blur itself. But you're right about the overlay the :dark value creates, it also seems to be a bad option.
I ended up updating the :blur-amount to 32 to get this:
It's still slightly bluish than the design but it's gonna be hard to eliminate the blue tone simply because the button itself is blue and it's getting blurred instead of disappearing. But at least the amount of blur seems to suit better and look closer to the expected result.
There was a problem hiding this comment.
It's still slightly bluish than the design but it's gonna be hard to eliminate the blue tone simply because the button itself is blue and it's getting blurred instead of disappearing.
It should not disappear. I think it is a design bug, I checked figma file and it don't have continue button in background.
- Little curious, why screenshot you shared don't have border-radius as screenshots in issue have. (unrelated)
- Please use another on-boarding screen or activity center for finding right value. It seems this screen is not right in figma itself.
There was a problem hiding this comment.
Please use another on-boarding screen or activity center for finding right value. It seems this screen is not right in figma itself.
If you think, the same value also works best for other screen, then I guess we can keep this fix for now. Thank you for fixing it.
cammellos
added
skip-manual-qa
design-review-required
and removed
request-manual-qa
labels
|
changed back to manual qa till waiting response from @alwx |
|
@Parveshdhull @flexsurfer to be honest, I don't think that blur-related issues need to be addressed somewhat separately — we know that the blur almost never looks like the designs and it's more of an issue with the component itself. My proposal would be to update the blur amount as part of this issue and create another umbrella issue for all blur-related things. |
There was a problem hiding this comment.
Thank you for fixing it. Please make sure this :blur-amount is working also for other screens mentioned in issue. (As onboarding screen is not good for comparing due to design bug)
94% of end-end tests have passedFailed tests (2)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (45)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestDeepLinksOneDevice:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
|
@alwx e2e are fine here. If PR does not require manual qa then it is ready for design review. cc @Francesca-G |
@pavloburykh I was trying to install the PR but this is where the QR code brings me 
could you please check if it's a problem on my side? Thank you 🙏 |
pavloburykh
force-pushed
the
bugfix/18924
branch
from
2fc3c95 to
3c22f3a
Compare
Hey @Francesca-G! IOS build was outdated. I rebased and updated the builds. Here is the latest IOS build https://i.diawi.com/cNyXgg |
|
@alwx can you please merge it? |
Fixes #18924
Screenshot:
Platforms
status: ready