Image index fix #15525
Image index fix #15525
Conversation
Jenkins BuildsClick to see older builds (4)
|
| @@ -130,7 +130,7 @@ | |||
| ;; The initial value of data is the image that was pressed (and not the whole album) in order | |||
| ;; for the transition animation to execute properly, otherwise it would animate towards | |||
| ;; outside the screen (even if we have `initialScrollIndex` set). | |||
| data (reagent/atom [(nth messages index)]) | |||
| data (reagent/atom (if (number? index) [(nth messages index)] [])) | |||
There was a problem hiding this comment.
Interestingly, similar to what we have in other functions like safe-trim, safe-replace, we could have safe-nth, so we can reduce the chance of this kind of bug in the future, which is quite easy to happen again, since nth doesn't accept nil and many things can be nil in Clojure.
There was a problem hiding this comment.
@ilmotta that's right.
But here I do not want to call nth at all if index is not number. So if we have something like:
(defn safe-nth
[coll index]
(when (number? index)
(nth coll index)))
And then use it as: (reagent/atom [(utils/safe-nth messages index)])
If index is not a number, the atom will become [nil], but I want it to be [].
There was a problem hiding this comment.
Here you can evolve the function to respect the last argument of nth, which is the default value.
Something like this. It's also interesting to name the arguments just like the core function nth does.
(defn safe-nth
([coll n]
(safe-nth coll n nil))
([coll n not-found]
(if (number? n)
(nth coll n not-found)
not-found)))So now you call it:
(safe-nth messages index [])There was a problem hiding this comment.
Looks good! Thanks @ilmotta
There was a problem hiding this comment.
@ilmotta but this will make it [[]], right?
(reagent/atom [(utils/safe-nth messages index [])])
There was a problem hiding this comment.
So maybe what you want is different @OmarBasem.
(into [] [1 2 3]) ; => [1 2 3]
(into [] []) ; => []
(into [] nil) ; => []
;; No need for the default value.
(reagent/atom (into [] (utils/safe-nth messages-index)))Please double-check, I'm not 100% the code above works.
There was a problem hiding this comment.
@ilmotta so this works (into [] (safe-nth [1 2] nil)) -> []
and this works (safe-nth [1 2] 0) -> 1
but this does not work (into [] (safe-nth [1 2] 0)) -> Execution error (IllegalArgumentException) at user/eval146 (REPL:1). Don't know how to create ISeq from: java.lang.Long
I think I can just keep it as an if condition :)
There was a problem hiding this comment.
Damn, you're right, the wheel keeps turning and going back to the original code. into likes sequences.
93% of end-end tests have passedFailed tests (2)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Passed tests (27)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
|
|
hi, @OmarBasem thank you for the fix. No issues from my side. PR is ready o be merged |
fixes: #15391