fix: use blacklist for parallax #18971
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cammellos
approved these changes
Feb 23, 2024
Jenkins BuildsClick to see older builds (24)
|
ilmotta
approved these changes
Feb 23, 2024
| (:require | ||
| [native-module.core :as native-module])) | ||
|
|
||
| (def ^:const device-id (:device-id (native-module/get-device-model-info))) |
There was a problem hiding this comment.
Nitpick: we don't need to use :const metadata as much as we think we need. The only difference is that the CLJS compiler will inline device-id whenever it sees it. But device-id is currently being used only by the static value blacklisted?. Same for minimum-device-code. So it ends up being noise for no tangible value.
|
|
||
| (defn get-model-code | ||
| [model] | ||
| (let [match (re-find #"iPhone(\d+)" model)] |
There was a problem hiding this comment.
You can destructure to simplify:
(let [[_ version] (re-find #"iPhone(\d+)" "iPhone11")]
version) ; => "11"|
|
||
| (def ^:const minimum-device-code 11) | ||
|
|
||
| (def blacklisted? |
There was a problem hiding this comment.
We can have all other vars except this one made private.
BalogunofAfrica
force-pushed
the
fix/use-blacklist-for-parallax
branch
2 times, most recently
from
153c387
to
660e308
Compare
12% of end-end tests have passedFailed tests (41)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (6)Click to expandClass TestDeepLinksOneDevice:
Class TestCommunityOneDeviceMerged:
|
2 tasks
BalogunofAfrica
force-pushed
the
fix/use-blacklist-for-parallax
branch
2 times, most recently
from
25acca8
to
84c19d0
Compare
85% of end-end tests have passedFailed tests (6)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (41)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestDeepLinksOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
BalogunofAfrica
force-pushed
the
fix/use-blacklist-for-parallax
branch
from
84c19d0
to
a2fe6a8
Compare
85% of end-end tests have passedFailed tests (6)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (41)Click to expandClass TestDeepLinksOneDevice:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
|
17% of end-end tests have passedFailed tests (5)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Passed tests (1)Click to expandClass TestCommunityOneDeviceMerged:
|
|
Hi @BalogunofAfrica ! |
BalogunofAfrica
force-pushed
the
fix/use-blacklist-for-parallax
branch
from
a2fe6a8
to
86dbbe0
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use blacklist for determining device to show parallax animation to.
This solves the issue as noted here https://discord.com/channels/1103692771585433630/1103692773363810317/1210598484076535838 which was earlier raised by John
status: ready