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.

Sign up for GitHub

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 first tap doesn't work when selecting a photo in the gallery #18885

Merged
merged 1 commit into from
Feb 19, 2024
Merged

fix first tap doesn't work when selecting a photo in the gallery #18885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/status_im/common/bottom_sheet_screen/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
[content
{:insets insets
:close close
:scroll-enabled? @scroll-enabled?
:scroll-enabled? scroll-enabled?
Copy link
Member Author

Choose a reason for hiding this comment

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

Bug cause: Early deref rerendered parent and reinitialized states in child components

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh so this was the side effect that i was worried about it, my bad. thank you @Parveshdhull

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm but passing a ratom is not ideal, no? Then the component unnecessarily needs to be aware that the prop is a ratom

Copy link
Member Author

Choose a reason for hiding this comment

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

We can use some naming convention to clarify that the passed prop is ratom.

:current-scroll curr-scroll
:on-scroll #(on-scroll % curr-scroll)
:sheet-animating? animating?}]]]]))))
Expand Down
2 changes: 1 addition & 1 deletion src/status_im/common/emoji_picker/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
on-select set-scroll-ref close sheet-animating?]}]
[gesture/flat-list
{:ref set-scroll-ref
:scroll-enabled scroll-enabled?
:scroll-enabled @scroll-enabled?
:data (or filtered-data emoji-picker.data/flatten-data)
:initial-num-to-render 14
:max-to-render-per-batch 10
Expand Down
2 changes: 1 addition & 1 deletion src/status_im/contexts/chat/home/new_chat/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
:render-section-header-fn contact-list/contacts-section-header
:content-container-style {:padding-bottom 70}
:render-fn contact-item-render
:scroll-enabled scroll-enabled?
:scroll-enabled @scroll-enabled?
:on-scroll on-scroll}])
(when contacts-selected?
[quo/button
Expand Down
13 changes: 6 additions & 7 deletions src/status_im/contexts/chat/messenger/composer/actions/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,7 @@

(defn open-photo-selector
[{:keys [input-ref]}
{:keys [height]}
insets]
{:keys [height]}]
(permissions/request-permissions
{:permissions [(if platform/is-below-android-13? :read-external-storage :read-media-images)
:write-external-storage]
Expand All @@ -192,18 +191,18 @@
(.blur ^js @input-ref))
(rf/dispatch [:chat.ui/set-input-content-height
(reanimated/get-shared-value height)])
(rf/dispatch [:open-modal :photo-selector {:insets insets}]))
(rf/dispatch [:photo-selector/navigate-to-photo-selector]))
:on-denied (fn []
(alert.effects/show-popup (i18n/label :t/error)
(i18n/label
:t/external-storage-denied)))}))

(defn image-button
[props animations insets edit]
[props animations edit]
[quo/composer-button
{:on-press (if edit
#(js/alert "This feature is temporarily unavailable in edit mode.")
#(open-photo-selector props animations insets))
#(open-photo-selector props animations))
:accessibility-label :open-images-button
:container-style {:margin-right 12}
:icon :i/image}])
Expand All @@ -228,15 +227,15 @@
:icon :i/format}])

(defn view
[props state animations window-height insets {:keys [edit images]}]
[props state animations window-height {:keys [edit images]}]
(let [send-btn-opacity (reanimated/use-shared-value 0)
audio-btn-opacity (reanimated/interpolate send-btn-opacity [0 1] [1 0])]
[rn/view {:style style/actions-container}
[rn/view
{:style {:flex-direction :row
:display (if @(:recording? state) :none :flex)}}
[camera-button edit]
[image-button props animations insets edit]
[image-button props animations edit]
[reaction-button]
[format-button]]
[:f> send-button props state animations window-height images edit send-btn-opacity]
Expand Down
2 changes: 1 addition & 1 deletion src/status_im/contexts/chat/messenger/composer/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@
[gradients/view props state animations show-bottom-gradient?]
[link-preview/view]
[images/images-list]]
[:f> actions/view props state animations window-height insets subscriptions]]]]]))
[:f> actions/view props state animations window-height subscriptions]]]]]))

(defn f-composer
[props]
Expand Down
2 changes: 1 addition & 1 deletion src/status_im/contexts/chat/messenger/photo_selector/album_selector/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
:content-container-style {:padding-top 64
:padding-bottom 40}
:key-fn key-fn
:scroll-enabled scroll-enabled?
:scroll-enabled @scroll-enabled?
:on-scroll on-scroll
:style {:height window-height}}]]))

Expand Down
7 changes: 7 additions & 0 deletions src/status_im/contexts/chat/messenger/photo_selector/events.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
(ns status-im.contexts.chat.messenger.photo-selector.events
(:require
[re-frame.core :as re-frame]
[status-im.constants :as constants]
status-im.contexts.chat.messenger.photo-selector.effects
[utils.i18n :as i18n]
Expand Down Expand Up @@ -74,3 +75,9 @@
(when (and (< (count images) constants/max-album-photos)
(not (some #(= (:uri image) (:uri %)) images)))
{:effects.camera-roll/image-selected [image current-chat-id]})))

(re-frame/reg-event-fx :photo-selector/navigate-to-photo-selector
(fn []
{:fx [[:dispatch [:open-modal :photo-selector]]
[:dispatch [:photo-selector/get-photos-for-selected-album]]
[:dispatch [:photo-selector/camera-roll-get-albums]]]}))
19 changes: 9 additions & 10 deletions src/status_im/contexts/chat/messenger/photo_selector/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,14 @@
(let [customization-color (rf/sub [:profile/customization-color])
item-selected? (some #(= (:uri item) (:uri %)) @selected)]
[rn/touchable-opacity
{:on-press (fn []
(if item-selected?
(swap! selected remove-selected item)
(if (>= (count @selected) constants/max-album-photos)
(show-photo-limit-toast)
(swap! selected conj item))))
:accessibility-label (str "image-" index)}
{:on-press (fn []
(if item-selected?
(swap! selected remove-selected item)
(if (>= (count @selected) constants/max-album-photos)
(show-photo-limit-toast)
(swap! selected conj item))))
:allow-multiple-presses? true
Copy link
Member Author

Choose a reason for hiding this comment

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

Additional Improvement (more details)

:accessibility-label (str "image-" index)}
[rn/image
{:source {:uri (:uri item)}
:style (style/image window-width index)}]
Expand All @@ -91,8 +92,6 @@

(defn photo-selector
[{:keys [scroll-enabled? on-scroll current-scroll close] :as sheet}]
(rf/dispatch [:photo-selector/get-photos-for-selected-album])
(rf/dispatch [:photo-selector/camera-roll-get-albums])
Comment on lines -94 to -95
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved these calls to event

(let [album? (reagent/atom false)
customization-color (rf/sub [:profile/customization-color])
sending-image (into [] (vals (rf/sub [:chats/sending-image])))
Expand Down Expand Up @@ -134,7 +133,7 @@
:padding-bottom (+ (safe-area/get-bottom) 100)
:padding-top 64}
:on-scroll on-scroll
:scroll-enabled scroll-enabled?
:scroll-enabled @scroll-enabled?
:on-end-reached (fn []
(when (and (not loading?) has-next-page?)
(rf/dispatch [:photo-selector/camera-roll-loading-more true])
Expand Down
2 changes: 1 addition & 1 deletion src/status_im/contexts/communities/actions/addresses_for_permissions/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
:share-all-addresses? share-all-addresses?
:community-color color}
:content-container-style {:padding-horizontal 20}
:scroll-enabled scroll-enabled?
:scroll-enabled @scroll-enabled?
:on-scroll on-scroll
:key-fn :address
:data accounts}]
Expand Down