chore: small refactor to remove some uses of status-im in status-im2 #16358
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
J-Son89
commented
Jun 22, 2023
| @@ -304,7 +305,7 @@ | |||
| (re-frame/dispatch [:set-in [:wallet/recipient :searching] | |||
| :searching]) | |||
| (debounce/debounce-and-dispatch [:wallet.recipient/address-changed | |||
| (utils/safe-trim %)] | |||
| (utils.string/safe-trim %)] | |||
There was a problem hiding this comment.
moved safe-trim to utils at src/utils
J-Son89
commented
Jun 22, 2023
src/status_im/utils/http.cljs
Outdated
| @@ -190,7 +190,3 @@ | |||
| [url] | |||
| (string/lower-case (apply str (map filter-letters-numbers-and-replace-dot-on-dash (url-host url))))) | |||
|
|
|||
| (defn replace-port | |||
There was a problem hiding this comment.
replace-port only used in chat context, for now moved to utils in status_im2/contexts/chat.
There was a problem hiding this comment.
http will be moved to status-im2, so it's better to keep replace-port here
There was a problem hiding this comment.
ok cool, I can either add this to that pr or just remove the replace-port changes and do that in another pr. I will see what's involved.
Any idea where http stuff should go in the codebase?
There was a problem hiding this comment.
ah okay, thanks @flexsurfer I will sort this!
There was a problem hiding this comment.
so I removed the http migrations from this pr and will do it in a follow up as I believe it will be a much bigger pr and want to keep things as focused as possible 👍
J-Son89
commented
Jun 22, 2023
| @@ -116,16 +116,6 @@ | |||
| (gstring/format (str "%." places "f") amount) | |||
| (or (str amount) 0)))) | |||
|
|
|||
| (defn safe-trim | |||
There was a problem hiding this comment.
moved safe-trim and safe-replace to utils at src/utils
J-Son89
commented
Jun 22, 2023
| @@ -4,78 +4,63 @@ | |||
| [react-native.gesture :as gesture] | |||
| [react-native.reanimated :as reanimated] | |||
| [reagent.core :as reagent] | |||
| [status-im.utils.utils :as utils.utils] | |||
| [react-native.background-timer :as background-timer] | |||
There was a problem hiding this comment.
use background timer from react native tooling in root of src
J-Son89
commented
Jun 22, 2023
| pan-down? (> evt-translation-y 100) | ||
| pan-up? (< evt-translation-y -30)] | ||
| (cond | ||
| pan-down? (reset-translate-y translate-y) |
There was a problem hiding this comment.
removed use of comments and used named vars instead 👍
Jenkins BuildsClick to see older builds (15)
|
flexsurfer
reviewed
Jun 23, 2023
src/status_im/utils/http.cljs
Outdated
| @@ -190,7 +190,3 @@ | |||
| [url] | |||
| (string/lower-case (apply str (map filter-letters-numbers-and-replace-dot-on-dash (url-host url))))) | |||
|
|
|||
| (defn replace-port | |||
There was a problem hiding this comment.
http will be moved to status-im2, so it's better to keep replace-port here
| @@ -50,8 +50,8 @@ | |||
| colors/neutral-100-opa-70 | |||
| colors/neutral-100-opa-0) | |||
| {:keys [background-color opacity]} animations | |||
| uri (http/replace-port (:image content) | |||
| (rf/sub [:mediaserver/port]))] | |||
| uri (chat-utils/replace-port (:image content) | |||
There was a problem hiding this comment.
it's definitely http utils, not chat
|
@churik I will remove label for the moment as I have to make some small adjustments first. Thanks for adding it initially! :) |
OmarBasem
approved these changes
Jun 26, 2023
J-Son89
force-pushed
the
jc/refactor-1
branch
3 times, most recently
from
54fdf16
to
2fab8bf
Compare
flexsurfer
approved these changes
Jun 26, 2023
39% of end-end tests have passedFailed tests (20)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (13)Click to expandClass TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePR:
|
|
@J-Son89 can you please check lint? Thanks! |
|
thanks @churik - sorting now! |
61% of end-end tests have passedFailed tests (13)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Passed tests (20)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
|
82% of end-end tests have passedFailed tests (6)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Passed tests (27)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
|
|
@churik I can merge? :) |
|
@J-Son89 yes:) |
not at all, thanks! 🙌 |
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.
This pr includes a few small changes to remove the use of status_im requires within status-im2 namespace.
There are three small changes:
moved safe-trim and safe replace from
src/status_im/utils/utils.cljsto utils atsrc/utils/string.cljsupdated use of background-timer in toasts to use same feature from
react-native.background-timermoved
replace-portfunction insrc/status_im/utils/http.cljstosrc/status_im2/contexts/chat/utils/core.cljsas this function is only used in the context of chatTo test:
This adds no new functionality as it's just moving code around. But some good places to check are in chat / sending images.
Toasts and also wallet with views related to reciepients 👍