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

Design fixes for profile options #15966

Merged
merged 2 commits into from
May 22, 2023
Merged

Design fixes for profile options #15966

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
513dd16
design fixes
vkjr May 19, 2023
0f0caee
Use blur view that fallbacks to rn/view on Android
vkjr May 22, 2023
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
Binary file modified resources/images/icons2/20x20/[email protected]
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified resources/images/icons2/20x20/[email protected]
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion src/quo2/components/drawers/action_drawers/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
(defn- get-icon-color
[danger? override-theme]
(if danger?
colors/danger-50
colors/danger-60
(colors/theme-colors colors/neutral-50 colors/neutral-40 override-theme)))

(def divider
Expand Down
2 changes: 1 addition & 1 deletion src/status_im2/common/bottom_sheet/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
(styles/sheet insets window-height override-theme shell?))}

(when shell?
[blur/view
[blur/ios-view
Copy link
Member

Choose a reason for hiding this comment

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

Do we have issues for blur on Android here? Asking so I can try to fix it here #15953

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, with blur/view it looks incorrect on Android

Copy link
Contributor Author

Choose a reason for hiding this comment

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

photo_2023-05-22_11-45-43

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, I'll try to fix it on my PR 👍

{:style styles/shell-bg}])

(when selected-item
Expand Down
3 changes: 2 additions & 1 deletion src/status_im2/contexts/onboarding/profiles/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@
(defn show-profile-options
[key-uid context]
(rf/dispatch [:show-bottom-sheet
{:content (fn [] [profile-options key-uid context])}]))
{:content (fn [] [profile-options key-uid context])
:shell? true}]))

(defn profile-card
[{:keys [name key-uid customization-color keycard-pairing last-index set-hide-profiles]
Expand Down
7 changes: 6 additions & 1 deletion src/status_im2/navigation/view.cljs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,17 @@
(when js/goog.DEBUG
[reloader/reload-view])]))))

; Designs require bottom inset to be bigger than safe area, otherwise it is too close to the bottom
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think its better if we move this block of code to status-mobile/src/react_native/safe_area.cljs ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure, to be honest. Safe area is used in a lot of different contexts and the file safe_area.cljs used as an interface react-native-static-safe-area-insets. So I don't think that moving bottom-sheet stuff there will be beneficial. Whereas its declaration near the place of use looks more obvious to me.
Wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I can understand why you did this, to be honest in most cases i do this too.

The thought process behind my original comment was this ,
Since safe_area.cljs is an interface, I thought it would be better to have a bottom sheet variant over there which could then be used like this in your code like this :

(def bottom-sheet
  (reagent/reactify-component
   (fn []
     (let [{:keys [sheets hide?]} (rf/sub [:bottom-sheet])
           sheet                  (last sheets)
           insets                 (safe-area/bottom-sheet-insets)]
       ^{:key (str "sheet" @reloader/cnt)}
       [:<>
        [inactive]

But totally upto you, I don't have a very strong opinion on this :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked all the usages of (safe-area/get-insets) and didn't find other similar code where insets are changed right after getting. So I would suggest leaving it as is. And if we will find out that different components also want to get some alternated insets, we can move all these alternations to safe_area.cljs.

In general I think that developer who opened safe_area.cljs for his own reasons shouldn't stumble upon bottom-sheet mention and think about it, since it is on another level of abstraction)

(defn bottom-sheet-insets
[]
(assoc (safe-area/get-insets) :bottom 55))

(def bottom-sheet
(reagent/reactify-component
(fn []
(let [{:keys [sheets hide?]} (rf/sub [:bottom-sheet])
sheet (last sheets)
insets (safe-area/get-insets)]
insets (bottom-sheet-insets)]
^{:key (str "sheet" @reloader/cnt)}
[:<>
[inactive]
Expand Down