Skip to content

[#22580] Help community owners to make their community visible #22590

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.

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

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

ulisesmac
Copy link
Contributor

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-after based on delay-render that 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:

  • 😄 It's a development build, so a prod. build should perform even better.
  • 😞 It's running on a Galaxy Z fold 6, a high-end Android phone, on Emulators it also looked very smooth, and that's also my expectation on iOS, but I don't expect something great on low-end Android phones.
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

  • Android
  • iOS

status: ready

@ulisesmac ulisesmac requested review from ilmotta and seanstrom May 16, 2025 06:58
@ulisesmac ulisesmac self-assigned this May 16, 2025
@status-github-bot-v2 status-github-bot-v2 bot moved this to CONTRIBUTOR in Pipeline for QA May 16, 2025
@ulisesmac ulisesmac moved this from CONTRIBUTOR to REVIEW in Pipeline for QA May 16, 2025
Comment on lines 28 to +29
(defn- content
[{:keys [theme type button-label on-button-press message]}]
[{:keys [theme type button-label on-button-press message button-icon-right]}]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We needed an icon for the info-box's button (:i/external), here it adds the parameter

@status-im-auto
Copy link
Member

status-im-auto commented May 16, 2025

Jenkins Builds

Commit #️⃣ Finished (UTC) Duration Platform Result
164dc62 #1 2025-05-16 07:02:53 ~3 min tests 📄log
✔️ 164dc62 #1 2025-05-16 07:09:02 ~9 min android-e2e 🤖apk 📲
✔️ 164dc62 #1 2025-05-16 07:09:28 ~10 min android 🤖apk 📲
✔️ 164dc62 #1 2025-05-16 07:11:27 ~12 min ios 📱ipa 📲
f25f028 #2 2025-05-16 07:22:17 ~3 min tests 📄log
✔️ f25f028 #2 2025-05-16 07:26:00 ~7 min android-e2e 🤖apk 📲
✔️ f25f028 #2 2025-05-16 07:27:48 ~9 min android 🤖apk 📲
✔️ f25f028 #2 2025-05-16 07:30:05 ~11 min ios 📱ipa 📲

Comment on lines +198 to +201
(defn render-after
[{:keys [_ms] :as opts} & children]
(delay-render (into [:<>] children) opts))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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).

Comment on lines +22 to 25
;; 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)})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The safe area fix on Android 👀 the community-cover image now is bigger and looks better :)

Comment on lines +76 to +81
(re-frame/reg-sub
:communities/community-members-count
(fn [[_ community-id]]
(re-frame/subscribe [:communities/community-members community-id]))
(fn [members _]
(count members)))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

checking for owner, so we add the info-box

Comment on lines -23 to +35
;; 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)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: REVIEW
Development

Successfully merging this pull request may close these issues.

Help community owners to make their community visible
2 participants