[#22580] Help community owners to make their community visible #22590
Conversation
| (defn- content | ||
| [{:keys [theme type button-label on-button-press message]}] | ||
| [{:keys [theme type button-label on-button-press message button-icon-right]}] |
There was a problem hiding this comment.
We needed an icon for the info-box's button (:i/external), here it adds the parameter
Jenkins BuildsClick to see older builds (8)
|
src/react_native/core.cljs
Outdated
| (defn render-after | ||
| [{:keys [_ms] :as opts} & children] | ||
| (delay-render (into [:<>] children) opts)) | ||
|
|
There was a problem hiding this comment.
This is expected to be used as a component (delay-render is used as a function), also it receives any amount of children, compared to delay-render that receives a single child.
Sometimes we want to delay a bit more the components, setTimeout with 0 is extremely fast, since our current React version on Android don't batch updates, an option would be use reagent/next-tick or js/RequestAnimationFrame instead.
Since some components need a bigger delay, this render-after component receives an ms key and renders after that period of time. (ms used to match the dispatch-later key from re-frame, so I don't introduce a new concept).
There was a problem hiding this comment.
Since you provided some useful context here, could you add a docstring to render-after as well @ulisesmac?
delay-renderis used as a function
delay-render in the repo is sometimes used as a function and sometimes as a component. Why do you want render-after to use delay-render as a function?
| ;; On Android we count the navigation bar page-nav padding top | ||
| ;; because it isn't overlapped with the safe-area top. | ||
| (when platform/android? 12) | ||
| safe-area/top)}) |
There was a problem hiding this comment.
The safe area fix on Android 👀 the community-cover image now is bigger and looks better :)
| (re-frame/reg-sub | ||
| :communities/community-members-count | ||
| (fn [[_ community-id]] | ||
| (re-frame/subscribe [:communities/community-members community-id])) | ||
| (fn [members _] | ||
| (count members))) |
There was a problem hiding this comment.
This was causing a re-render when visiting a community, now it's cached so we are saving one re-render.
As far as I tested, the screen doesn't re-render by external factors not related.
| :tags tags | ||
| :permissions permissions | ||
| :role-permissions? role-permissions?}))) | ||
| (let [owner? (= memberRole constants/community-member-role-owner)] |
There was a problem hiding this comment.
checking for owner, so we add the info-box
| ;; NOTE: values compared against `scroll-amount` to trigger animations. | ||
| (def expand-header-threshold | ||
| "Dragging distance to collapse/extend the community." | ||
| 150) | ||
|
|
||
| (def sheet-displacement-threshold | ||
| "Dragging distance to round sheet borders and move the sheet 8 units." | ||
| (+ expand-header-threshold 20)) | ||
| (def snap-header-threshold-factor | ||
| "Threshold to automatically move the header to a collapsed/expanded state and avoid an | ||
| intermediate state. Applied to `collapse-threshold`." | ||
| 0.65) | ||
|
|
||
| (def text-movement-threshold | ||
| "Dragging distance to start the text movement from/to the bottom to/from the right." | ||
| (* expand-header-threshold 0.7)) | ||
| (def navbar-content-threshold-factor | ||
| "When the community name and logo start to appear. Applied to sheet-displacement-threshold." | ||
| 32) | ||
|
|
||
| (def info-opacity-threshold | ||
| (def info-opacity-threshold-factor | ||
| "Dragging distance to appear/disappear the community info (description, tags & stats)." | ||
| (* expand-header-threshold 0.5)) | ||
|
|
||
| (def snap-header-threshold | ||
| "Threshold to automatically move the header to a collapsed/expanded state and avoid an | ||
| intermediate state." | ||
| (* expand-header-threshold 0.75)) | ||
|
|
||
| (def expand-header-limit | ||
| "Max dragging distance where the header animation ends. It works to identify when to | ||
| start the flat-list scrolling." | ||
| (+ sheet-displacement-threshold 56)) | ||
| 0.5) |
There was a problem hiding this comment.
These values were fixed, they implied a constant scroll gesture to collapse the community. but sometimes it felt strange, because a height of 500 was covered with a gesture of 150.
This PR had to move these values to be dynamic, that's why this is changing too much and also the worklets. The benefit is that now the gesture is 1:1 and feels very good
ulisesmac
force-pushed
the
22580-help-owners-make-their-community-visible
branch
from
f25f028 to
6250a1c
Compare
There was a problem hiding this comment.
@ulisesmac PR LGTM overall. Also thanks for the code improvements.
The only thing I'm not sure is the UX in iOS. As you can see in the video, it looks worse and always has a long and noticeable delay before the images load. Can you see the same issue?
The Android PR feels slightly better than develop because the flashing is less noticeable, but I had to rewatch multiple times to perceive. This made me question the need for the custom render-after component and if it's worth the trouble, but perhaps the improvements are more noticeable in other Android devices.
Edit: Android video recorded in a Galaxy Tab
Android 2.33
Screen_Recording_20250519_074857_Status.mp4
Android PR
Screen_Recording_20250519_075005_Status.PR.mp4
iOS PR
ScreenRecording_05-19-2025.19-42-12_1.MP4
src/react_native/core.cljs
Outdated
| (defn render-after | ||
| [{:keys [_ms] :as opts} & children] | ||
| (delay-render (into [:<>] children) opts)) | ||
|
|
There was a problem hiding this comment.
Since you provided some useful context here, could you add a docstring to render-after as well @ulisesmac?
delay-renderis used as a function
delay-render in the repo is sometimes used as a function and sometimes as a component. Why do you want render-after to use delay-render as a function?
| {:top (- safe-area/top 12) ;; -12 to place the button next to the safe-area | ||
| {:top (if platform/android? | ||
| safe-area/top | ||
| (- safe-area/top 12)) ;; -12 to place the button next to the safe-area |
There was a problem hiding this comment.
There was a problem hiding this comment.
I remember this was fine, and that ; was just for comments aligned to the right. I fixed these in the file. Thanks for the reminder,
|
@ilmotta Thank you so much for taking time to test it on Android! Regarding to the comparison with 2.33 on Android. The current implementation of the community overview is HEAVIER compared to 2.33, in 2.33 we don't handle gestures, the styles are wrong, there's no way to collapse and expand the community, etc. So, if Actually is awesome to know that the current impl. that involves more features and is heavier renders faster than the version with less features. About iOS..., sadly, I wasn't able to test it, I don't have access to my Macbook 😞 so it's going to be harder to solve it, I'll ask for help within the team if I don't find a solution |
|
Ok, now I saw the iOS video, my linux has problems playing it. It's very strange to me 🤔 I'm surprised that iOS renders everything SUPER FAST, I'd actually doubt you tested this PR in that video 🤔 In the video, the delay to display the images is not related to the So, what's going on?
**How to improve it?**🤔
I'd prefer to explore these approaches when I have access to my dev environment for iOS so I can properly address it, or if it's too bad, I'd ask for help within the team. |
I'm curious to see how other folks with iOS see the Status community loading and if they always see the banner flash slowly like it does for me. Maybe you can ask a QA or two to check in their iPhones. The problem does not happen in the other communities @ulisesmac, only the Status community. Would it be a deal breaker to release with this problem only for the Status community in iOS? I don't think so, as long as we can commit to fix in a subsequent release. Currently, in 2.33, it looks even worse IMO because there are a few animation artifacts when the Status community opens which you fixed in this PR. |
|
These are the white spaces I mentioned before, when the app don't have access to the images: 
And an important update on my previous comment: It's a more general issue! Look at this video (running 2.33) trim.0DAF14CB-5420-4192-A0A3-B707BC9A49C5.MOVEven the "discover communities" screen has that problem with its images when we navigate, the status community also presents it, other communities render faster and don't show the issue. So maybe we can investigate it separately and provide a general solution. WDYT? |
I understand, but I had tested your PR twice, today, and before in another day where I couldn't finish my PR review. I have also seen all the issues you recorded above regarding the Status community. Yes, definitely something about the Status community makes things worse. |
I agree, I shared above as well a few minutes ago, maybe you missed.
|
Update thresholds for smoother interaction
ulisesmac
force-pushed
the
22580-help-owners-make-their-community-visible
branch
from
6250a1c to
62a23c0
Compare
ulisesmac
force-pushed
the
22580-help-owners-make-their-community-visible
branch
from
62a23c0 to
45c4904
Compare
|
@ilmotta Comments addressed! |
| return useAnimatedStyle(() => { | ||
| const firstDisplacement = scrollAmount.value < expandHeaderThreshold; | ||
| if (firstDisplacement) { | ||
| const isFirstDisplacement = initialState.value === 'expanded' || scrollAmount.value < collapseThreshold.value; |
There was a problem hiding this comment.
Tiny comment: Maybe we can create a helper function for computing isFirstDisplacement?
| if (initialState.value === 'collapsed') { | ||
| opacity = 0; | ||
| } else if (initialState.value === 'expanded') { | ||
| opacity = 1; |
There was a problem hiding this comment.
Not sure who is choosing the names for collapsed or expanded (whether it's us or a library), but maybe we can introduce some constants for the strings?
| ([content] | ||
| (delay-render {:ms 0} content)) | ||
| ([{:keys [ms]} & children] | ||
| (let [[render? set-render] (use-state false)] | ||
| (use-mount | ||
| (fn [] | ||
| (js/setTimeout #(set-render true) ms))) | ||
| (when render? | ||
| (into [:<>] children))))) |
92% of end-end tests have passedFailed tests (4)Click to expandClass TestWalletCollectibles:
Class TestWalletConnectSignTransactions:
Class TestFallbackMultipleDevice:
Class TestWalletOneDevice:
Expected to fail tests (3)Click to expandClass TestWalletConnectSignTransactions:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedThree:
Passed tests (81)Click to expandClass TestWalletConnectLoggedOut:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestWalletOneDeviceTwo:
Class TestProfileMultipleDevices:
Class TestWalletCustomParamOneDevice:
Class TestDeepLinksOneDevice:
Class TestAndroid12:
Class TestWalletOneDeviceThree:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestAndroid13:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedThree:
Class TestWalletConnectSignTransactions:
Class TestActivityMultipleDevicePRTwo:
Class TestWalletCollectibles:
Class TestFallbackMultipleDevice:
Class TestWalletConnectBaseChecks:
Class TestProfileOneDevice:
Class TestWalletMultipleDevice:
Class TestWalletOneDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestWalletConnectDifferentNetworks:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
|
fixes #22580
Summary
This PR adds an info-box for community owners that helps them to increase the visibility of their community.
Review notes
Additionally, this PR improved the performance UI and UX for the community overview screen. why? Because I previously reworked this screen but didn't have enough time to have things on shape, since now this screen has been modified to add the info-box, it was a good moment to improve it.
This PR:
✅ Fixed the safe-area top on Android devices
✅ Improved the perceived rendering time by delaying some components (introduced
render-afterbased ondelay-renderthat accepts an amount of ms to delay the rendering).E.g. The cover image is rendered first (this avoids pushing a white screen to the user), then the navigation bar, the logo, and 150ms later (in the middle of the opening animation that lasts 300ms), the flat-list with the channels.
✅ Fixed the interaction when scrolling to the collapsed/expanded versions, previously it was hardcoded as a gesture of
150, now the user actually scrolls on the real height and it feels better.Check this video testing on a real device:
community-overview-demo.mp4
Testing notes
As long as this PR doesn't break the community overview (e.g. on iOS or on certain Android devices) I think the PR review can focus on and the actual feature: the info-box.
Platforms
status: ready