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 incorrect background color in Community Overview screen #16348

Merged
merged 1 commit into from
Jun 22, 2023
Merged

Fix incorrect background color in Community Overview screen #16348

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
26 changes: 14 additions & 12 deletions src/quo2/components/navigation/page_nav.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@
:justify-content :flex-end)}
(let [last-icon-index (-> right-section-buttons count dec)]
(map-indexed (fn [index
{:keys [icon on-press type style icon-override-theme accessibility-label label]
{:keys [icon on-press type style icon-override-theme icon-background-color
accessibility-label label]
:or {type :grey}}]
^{:key index}
[rn/view
Expand All @@ -162,12 +163,13 @@
accessibility-label (assoc :accessibility-label accessibility-label
:accessible true))
[button/button
{:on-press on-press
:icon (not label)
:type type
:before (when label icon)
:size 32
:override-theme icon-override-theme}
{:on-press on-press
:icon (not label)
:type type
:before (when label icon)
:size 32
:override-theme icon-override-theme
:override-background-color icon-background-color}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI: I used the exact same name icon-background-color that's used for the left-section-view component in this file. The prop was essentially missing.

(if label label icon)]])
right-section-buttons))])

Expand All @@ -179,26 +181,26 @@
:align-mid? true/false
:page-nav-color color
:page-nav-background-uri image-uri
:mid-section
:mid-section
{:type one-of :text-only :text-with-two-icons :text-with-one-icon :text-with-description :user-avatar
:icon icon
:main-text string
:left-icon icon
:right-icon icon
:left-icon icon
:right-icon icon
:description string
:description-color color
:description-icon icon
:description-user-icon icon
:description-img a render prop which will be used in place of :description-user-icon
:main-text-icon-color color
}
:left-section
:left-section
{:type button-type
:on-press event
:icon icon
:icon-override-theme :light/:dark
}
:right-section-buttons vector of
:right-section-buttons vector of
{:type button-type
:on-press event
:icon icon
Expand Down
45 changes: 20 additions & 25 deletions src/status_im2/common/scroll_page/style.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,6 @@
colors/neutral-95-opa-70)
:transparent)}))

(defn scroll-view-container
[border-radius]
{:flex 1
:position :absolute
:top (if platform/ios? -48 0)
:left 0
:right 0
:overflow :scroll
:border-radius border-radius})

(defn sticky-header-title
[animation]
(reanimated/apply-animations-to-style
Expand All @@ -55,24 +45,29 @@
:height 24
:margin-right 8})

(defn children-container
[{:keys [border-radius background-color]}]
{:flex 1
:border-top-left-radius border-radius
:border-top-right-radius border-radius
:background-color background-color})

(def picture-radius 40)
(def picture-diameter (* 2 picture-radius))
(def picture-border-width 4)

(defn display-picture-container
[animation]
(reanimated/apply-animations-to-style
{:transform [{:scale animation}]}
{:border-radius 50
:border-width 1
:border-color colors/white
:position :absolute
:top -40
:left 17
:padding 2
:background-color (colors/theme-colors
colors/white
colors/neutral-90)}))
{:border-radius picture-diameter
:border-width picture-border-width
:border-color (colors/theme-colors colors/white colors/neutral-95)
:position :absolute
:top (- (+ picture-radius picture-border-width))
:left (- (/ picture-radius 2) picture-border-width)}))

(def display-picture
{:border-radius 50
:border-width 0
:border-color :transparent
:width 80
:height 80})
{:border-radius picture-diameter
:width picture-diameter
:height picture-diameter})
8 changes: 3 additions & 5 deletions src/status_im2/common/scroll_page/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,7 @@
[:f> f-scroll-page-header @scroll-height height name page-nav-right-section-buttons
logo sticky-header top-nav title-colum navigate-back?]
[rn/scroll-view
{:content-container-style (style/scroll-view-container
(diff-with-max-min @scroll-height 16 0))
{:content-container-style {:flex-grow 1}
:shows-vertical-scroll-indicator false
:scroll-event-throttle 16
:on-scroll (fn [^js event]
Expand All @@ -140,9 +139,8 @@
:flex 1}}]])
(when children
[rn/view
{:flex 1
:border-radius (diff-with-max-min @scroll-height 16 0)
:background-color background-color}
{:style (style/children-container {:border-radius (diff-with-max-min @scroll-height 16 0)
:background-color background-color})}
(when cover-image
[:f> f-display-picture @scroll-height logo])
children])]])))
25 changes: 14 additions & 11 deletions src/status_im2/contexts/communities/overview/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
[status-im2.common.home.actions.view :as actions]
[status-im2.common.password-authentication.view :as password-authentication]
[status-im2.common.scroll-page.view :as scroll-page]
[status-im2.common.scroll-page.style :as scroll-page.style]
[status-im2.constants :as constants]
[status-im2.contexts.communities.actions.community-options.view :as options]
[status-im2.contexts.communities.overview.style :as style]
Expand Down Expand Up @@ -250,7 +251,9 @@
:ellipsize-mode :tail
:weight :semi-bold
:size :heading-1
:style {:margin-top 56}}
:style {:margin-top (+ scroll-page.style/picture-radius
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI: I parameterized certain style values. I think this practice of magical values is one of the sources of constant bugs in the UI. The question I ask myself to decide if we need vars is: if somebody changes the style value, will they remember to change this magical number? If the answer is no, then ideally, vars/constants should be used.

scroll-page.style/picture-border-width
12)}}
name])

(defn community-description
Expand Down Expand Up @@ -301,13 +304,13 @@

(defn page-nav-right-section-buttons
[id]
[{:icon :i/options
:background-color (scroll-page/icon-color)
:accessibility-label :community-options-for-community
:on-press #(rf/dispatch
[:show-bottom-sheet
{:content (fn []
[options/community-options-bottom-sheet id])}])}])
[{:icon :i/options
:icon-background-color (scroll-page/icon-color)
:accessibility-label :community-options-for-community
:on-press #(rf/dispatch
[:show-bottom-sheet
{:content (fn []
[options/community-options-bottom-sheet id])}])}])

(defn pick-first-category-by-height
[scroll-height first-channel-height categories-heights]
Expand Down Expand Up @@ -336,7 +339,7 @@
:name name
:on-scroll #(reset! scroll-height %)
:navigate-back? true
:background-color (colors/theme-colors colors/white colors/neutral-90)
:background-color (colors/theme-colors colors/white colors/neutral-95)
:height (if platform/ios? 100 148)}
[sticky-category-header
{:enabled (> @scroll-height @first-channel-height)
Expand All @@ -350,8 +353,8 @@
pending?
{:on-category-layout (partial add-category-height categories-heights)
:on-first-channel-height-changed
;; Here we set the height of the component
;; and we filter out the categories, as some might have been removed
;; Here we set the height of the component and we filter out the
;; categories, as some might have been removed
(fn [height categories]
(swap! categories-heights select-keys categories)
(reset! first-channel-height height))}]]))))
Expand Down