[#21233] Add missing community stats #22143
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ulisesmac
requested review from
ilmotta,
seanstrom,
flexsurfer and
Parveshdhull
ulisesmac
commented
Feb 17, 2025
Comment on lines
-20
to
-37
| (defn card-stats-container | ||
| [] | ||
| (def card-stats-container | ||
| {:flex-direction :row}) | ||
|
|
||
| (defn list-stats-container | ||
| [] | ||
| (def list-stats-container | ||
| {:flex-direction :row | ||
| :align-items :center}) | ||
|
|
||
| (defn card-stats-position | ||
| [] | ||
| (def card-stats-position | ||
| {:position :absolute | ||
| :top 116 | ||
| :right 12 | ||
| :left 12}) | ||
|
|
||
| (defn community-tags-position | ||
| [] |
There was a problem hiding this comment.
These styles didn't need to be a function
ulisesmac
commented
Feb 17, 2025
Comment on lines
-103
to
-105
| :on-layout #(swap! view-size | ||
| (fn [_] | ||
| (- (oops/oget % "nativeEvent.layout.width") 40)))} |
There was a problem hiding this comment.
Removed this swap! with no args to use a reset!
Jenkins BuildsClick to see older builds (8)
|
ulisesmac
commented
Feb 17, 2025
Comment on lines
133
to
-160
| (defn other-communities-list | ||
| [{:keys [communities communities-ids view-type]}] | ||
| [{:keys [communities view-type]}] | ||
| [rn/view {:style style/other-communities-container} | ||
| (if (and communities (pos? (count communities))) | ||
| (map-indexed | ||
| (fn [inner-index item] | ||
| (let [community-id (when communities-ids item) | ||
| community (if communities | ||
| item | ||
| (rf/sub [:communities/home-item community-id])) | ||
| cover {:uri (get-in (:images item) [:banner :uri])}] | ||
| [rn/view | ||
| {:key (str inner-index (:id community)) | ||
| :margin-bottom 16} | ||
| (if (= view-type :card-view) | ||
| [quo/community-card-view-item | ||
| {:community (assoc community :cover cover) | ||
| :on-press #(rf/dispatch [:communities/navigate-to-community-overview (:id community)])}] | ||
|
|
||
| [quo/community-list | ||
| {:on-press (fn [] | ||
| (rf/dispatch [:dismiss-keyboard]) | ||
| (rf/dispatch [:communities/navigate-to-community-overview | ||
| (:id community)])) | ||
| :on-long-press #(js/alert "TODO: to be implemented")} | ||
| community])])) | ||
| (if communities communities communities-ids)) |
There was a problem hiding this comment.
Lots of issues in this function, communities-ids was never passed and the logicis not even prepared to use it if passed
There was a problem hiding this comment.
Hmm odd that with what's happening with communities-ids in the original version. And it seems as you say there's no other usages of this function.
Nice clean up, thank you! 🙌
ulisesmac
commented
Feb 17, 2025
Comment on lines
182
to
-205
| (defn render-communities | ||
| [selected-tab | ||
| featured-communities-count | ||
| featured-communities | ||
| view-type] | ||
| (fn [] | ||
| [rn/view | ||
| [discover-communities-header | ||
| {:selected-tab selected-tab | ||
| :view-type view-type | ||
| :featured-communities-count featured-communities-count | ||
| :featured-communities featured-communities}] | ||
| [communities-lists selected-tab view-type]])) | ||
| [{:keys [selected-tab featured-communities-count featured-communities view-type]}] | ||
| [rn/view | ||
| [discover-communities-header | ||
| {:selected-tab selected-tab | ||
| :view-type view-type | ||
| :featured-communities-count featured-communities-count | ||
| :featured-communities featured-communities}] | ||
| [communities-lists selected-tab view-type]]) | ||
|
|
||
| (defn render-sticky-header | ||
| [{:keys [selected-tab scroll-height]}] | ||
| (fn [] | ||
| (when (> @scroll-height 360) | ||
| [rn/view | ||
| {:style (style/blur-tabs-header (safe-area/get-top))} | ||
| [discover-communities-segments selected-tab true]]))) |
There was a problem hiding this comment.
These form-2 components created the issue with the theme not changing correctly
ulisesmac
force-pushed
the
21233-missing-community-stats
branch
from
59df5ec to
4af29a1
Compare
ilmotta
approved these changes
Feb 20, 2025
There was a problem hiding this comment.
LGTM @ulisesmac, no further comments. It's nice to see a little feature here and there 🚀 Once this PR is merged, it would be a good idea to update this entry in the Feature Parity database in Notion https://www.notion.so/Discover-communities-show-stats-1a08f96fb65c80699552c6a2fd946c46
Also fixes this bug with the theme change
I faced this bug a couple times because I change themes multiple times per day, sometimes 😅 Thanks for fixing that!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #21233
Summary
This PR solves the missing stats in the discover communities screen along with some code improvements.
Stats shown:
Also fixes this bug with the theme change:
theme-bug.mp4
Review notes
This screen has several issues in its implementation, worth evaluating to reafactor it, maybe rework the screen when the
openandgatedoptions are implemented?Platforms
status: ready