-
Notifications
You must be signed in to change notification settings - Fork 993
[#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
base: develop
Are you sure you want to change the base?
[#22580] Help community owners to make their community visible #22590
Conversation
Update thresholds for smoother interaction
(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.
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
Jenkins Builds
|
(defn render-after | ||
[{:keys [_ms] :as opts} & children] | ||
(delay-render (into [:<>] children) opts)) | ||
|
There was a problem hiding this comment.
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).
;; 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.
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 :)
(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.
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)] |
There was a problem hiding this comment.
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
;; 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.
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
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 ondelay-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:
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