'my' context and user settings in mobile #256

thomasgwatson · 2025-02-12T05:06:00Z

See #254 for the goals of this branch.

@lorenjohnson I'm trying to treat each commit on this branch as a diff worth discussing.

First one in, want to get your feel on the brute force approach for my context nav out of the tab

Will next focus on making sure the various my context menu links work

…hared module to be able to use i18n instance without passing t

thomasgwatson · 2025-02-12T05:31:23Z

Ok, one piece of possible weirdness:

In the front end, we don't have the static groups. And the front-end basically treats my and all as the same entity. So similarly.... we should probably collapse those two down to the same static group?

lorenjohnson · 2025-02-12T05:41:50Z

apps/mobile/src/navigation/TabsNavigator.js

@@ -77,7 +86,14 @@ export default function TabsNavigator () {
      />
      <Tabs.Screen
        name='Settings Tab'
-        component={UserSettingsTabsNavigator}
+        component={HomeNavigator} // it will never navigate to this but we need to pass a valid component here anyway


This looks fine by me for now.

lorenjohnson · 2025-02-12T05:45:05Z

Ok, one piece of possible weirdness:

In the front end, we don't have the static groups. And the front-end basically treats my and all as the same entity. So similarly.... we should probably collapse those two down to the same static group?

By front-end you mean Web. How does it relate to things here? I am not making the connection, but I do hear that My and All are the same, or mostly the same target. I think if they are not actually the same then keeping them as their own targets it still good, and to share their common functionality through components they both use, vs blending the logic. However I'm speaking on abstractly and not entirely familiar with the patterns yet. My includes all of All though I think you were telling me? But still have an /all route?

thomasgwatson · 2025-02-12T23:23:29Z

So yeah...

Post new context menu; my has basically subsumed all of all routes... so now I'm wondering if we don't just go the whole way and change all references to all to my.

On web, at the moment, while the context part of their paths are still different, both the my and all paths end up with the same context menu. And obviously display different things in the main panel based on the rest of their paths.

Its probably more trouble to keep both all and my than it will be to tidy up the remainder of all references.

@tibetsprague would like you to add your perspective; do we really need to have the /all context?

thomasgwatson · 2025-02-13T04:37:23Z

apps/mobile/src/navigation/UserSettingsTabsNavigator.js

@@ -72,7 +72,7 @@ export default function UserSettingsTabsNavigator ({ navigation, route }) {
    })
  }, [navigation, route])

-
+  console.log('does this get called? at alll?')


It doesn't, because the navigator isn't rendered

thomasgwatson · 2025-02-13T04:40:23Z

apps/mobile/src/navigation/UserSettingsTabsNavigator.js

@@ -85,42 +85,42 @@ export default function UserSettingsTabsNavigator ({ navigation, route }) {
        name='Edit Profile'
        component={UserSettingsWebView}
        initialParams={{
-          path: '/settings'
+          path: '/my/edit-profile'


I was trying to navigate to these webviews with the existing structure and haven't figured it out. But do they even need a separate settings tab/navigator now? Could these screens just be moved over to the home navigator?

thomasgwatson · 2025-02-13T04:41:11Z

apps/mobile/src/navigation/linking/index.js

  '/:groupSlug(all|public)/post/:id':                        `${AUTH_ROOT_SCREEN_NAME}/Drawer/Tabs/Home Tab/Post Details`,
  '/:groupSlug(all|public)/post/:id/comments/:commentId':    `${AUTH_ROOT_SCREEN_NAME}/Drawer/Tabs/Home Tab/Post Details`,
  '/:groupSlug(all)/members/:id':                            `${AUTH_ROOT_SCREEN_NAME}/Drawer/Tabs/Home Tab/Member`,
  '/:groupSlug(all)/topics/:topicName':                      `${AUTH_ROOT_SCREEN_NAME}/Drawer/Tabs/Home Tab/Stream`,
+  '/:groupSlug(all|public)/events':                          `${AUTH_ROOT_SCREEN_NAME}/Drawer/Tabs/Home Tab/Stream`,
+  '/:groupSlug(all|public)':                                 `${AUTH_ROOT_SCREEN_NAME}/Drawer/Tabs/Home Tab/Stream`,


Moved this to the bottom, so other all/public paths wouldn't be eaten by this one first

Correct! Always be on the lookout for that, the logic can seem a little inverted if thinking it through. This is always the correct pattern though. The least specific things last.

thomasgwatson · 2025-02-13T04:41:31Z

apps/mobile/src/navigation/linking/index.js

+  '/:groupSlug(my)/blocked-users':                           `${AUTH_ROOT_SCREEN_NAME}/Drawer/Tabs/Settings Tab/Blocked Users`,
+  '/:groupSlug(my)/:section?':                               `${AUTH_ROOT_SCREEN_NAME}/Drawer/Tabs/Settings Tab/Edit Profile`,
+  // So, some user settings aren't explicitly linked, but I assume are avaiable because of this section wildcard.
+  // But ultimately everything just redirects to the `Edit Profile` screen.... so maybe none of the extra paths need to be here?


apps/mobile/src/screens/UserSettingsWebView/UserSettingsWebView.js

thomasgwatson · 2025-02-13T04:46:58Z

@lorenjohnson

This is what openURL is translating the path to... and it looks right, as far as I can tell. But when its passed to return navigation.dispatch(actionForPath), it doesn't seem to do anything.

From the openURL debugging:

Got any ideas on what needs to be switched around here? Am I just barking up the wrong tree by trying to navigate to these screens after changing the path structure?

thomasgwatson · 2025-02-13T04:54:08Z

oooooooooo I have an idea....

I removed the Settings Tab from the Tabs navigator when I made the changes to the user avatar... will fiddle with that for a second

thomasgwatson · 2025-02-13T05:04:58Z

Ok, that kinda works. I'll keep playing with this when I get back from my break

tibetsprague · 2025-02-13T06:58:40Z

my perspective on /my and /all. The main reasons to keep /all are:

To support old links that use /all.
Because it becomes awkward to figure out the right URL to all posts across all your groups when it starts with /my. Would it be /my/all-group-posts? /my/all-group-events, etc? /all/posts is just cleaner and makes sense when you read it.

That being said I could be convinced to switch to /my for everything with a good proposal :)

lorenjohnson · 2025-02-13T16:58:40Z

my perspective on /my and /all. The main reasons to keep /all are:

To support old links that use /all.

Because it becomes awkward to figure out the right URL to all posts across all your groups when it starts with /my. Would it be /my/all-group-posts? /my/all-group-events, etc? /all/posts is just cleaner and makes sense when you read it.

That being said I could be convinced to switch to /my for everything with a good proposal :)

/my/groups/stream ?

…bile header

…arams processing in custom linking config

…gging logging and comments, cleans-up some code

Make i18n-next instance (not config or translations) a shared module

…t+user-settings-mobile-LEJ

…ms work

…com/Hylozoic/hylo into 254-my-context+user-settings-mobile

…ed data

…bile-LEJ 'my' context and user settings in mobile (LEJ)

sentry-io · 2025-02-22T08:30:29Z

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

‼️ React ErrorBoundary TypeError: Cannot read property 'slug' of undefined ChatRoomWebView(/Users/lorenjohnson/dev/hylo/hy... View Issue

_{Did you find this useful? React with a 👍 or 👎}

lorenjohnson and others added 3 commits February 3, 2025 16:19

chore(config): Adds @hylo/config to locate shared i18n instance for s…

f4adc7a

…hared module to be able to use i18n instance without passing t

Merge branch 'dev' into add-config-i18n-package

81b93a8

Avatar tab press changes to my context and opens drawer

718bb70

thomasgwatson requested a review from lorenjohnson February 12, 2025 05:06

thomasgwatson changed the title ~~'my' context and user~~ 'my' context and user settings in mobile Feb 12, 2025

Merge branch 'dev' into 254-my-context+user-settings-mobile

78f9e64

lorenjohnson reviewed Feb 12, 2025

View reviewed changes

thomasgwatson added 5 commits February 13, 2025 11:08

Merge branch 'dev' into 254-my-context+user-settings-mobile

acd1d84

sync mobile and web widgetUrl and friends

7e3a1e6

Remove merge orphaned component

073a730

Ensure mobile links my profle widget to consistent route

03020d0

Not sure why navigation to user settings isnt working now....

1e8be66

thomasgwatson commented Feb 13, 2025

View reviewed changes

apps/mobile/src/screens/UserSettingsWebView/UserSettingsWebView.js Outdated Show resolved Hide resolved

thomasgwatson added 5 commits February 14, 2025 06:49

Return UserSettings tab to tabs nav

ee0f261

Merge branch 'dev' into 254-my-context+user-settings-mobile

a0ed6c6

Merge branch 'dev' into 254-my-context+user-settings-mobile

8f66328

Fix unnecessary import

808d314

Fix some my routes in mobile; Add better title for my context in mo…

787c040

…bile header

lorenjohnson and others added 27 commits February 20, 2025 08:58

chore(presenters): Move routing related static widget props to front

889f168

chore(mobile): HomeTab screen organization

4de145d

chore(mobile): Remove unused myHome from Stream Query and setup

7766b10

chore(mobile): Organize HomeNavigator screen list

7c1d593

chore(mobile): Bring back group fetching awareness to ContextMenu

a2fe0ab

chore(mobile): Remove routeParams-based setToGroupSlug from DrawerMenu

36aa438

chore(mobile): Add "all" -> "my" context translation to screen path p…

5ba35ba

…arams processing in custom linking config

chore(mobile): Linking and StreamQuery alignments, useRouteParams fix

a9d318a

chore(mobile): Fixes bugs in Stream Query variables composition, debu…

7e0aaee

…gging logging and comments, cleans-up some code

chore(mobile): fixes issue with NewMessage

8071456

Merge pull request #284 from Hylozoic/add-config-i18n-package

51c8134

Make i18n-next instance (not config or translations) a shared module

Merge branch 'dev' into 254-my-context+user-settings-mobile

7a2b5c7

Merge branch '254-my-context+user-settings-mobile' into 254-my-contex…

5cb91bd

…t+user-settings-mobile-LEJ

chore(mobile): Updates to Stream Query Variables that make most strea…

99c76a5

…ms work

Merge branch 'dev' into 254-my-context+user-settings-mobile

ec2c963

Merge branch '254-my-context+user-settings-mobile' of https://github.…

47c694e

…com/Hylozoic/hylo into 254-my-context+user-settings-mobile

widgets to my/stream and my/map go to expected screen and load expect…

2893ddd

…ed data

Revert streamParams tweak

4c36355

Do not use /stream as a :myHome param

8f44c40

Add note to experimental /my routes

a7d0ca1

Adjust /my/profile to go to the non-modal screen

8ef6908

Make sure titles for widgets render after Presenter changes

88f760f

Remove comment, use openUrl for All view nav

26eba9e

Split decisions widget into Proposals and Moderation widgets

bf2f70d

Fix group member navigation

9c25337

Merge branch 'dev' into 254-my-context+user-settings-mobile-LEJ

e741079

Merge pull request #285 from Hylozoic/254-my-context+user-settings-mo…

cab091d

…bile-LEJ 'my' context and user settings in mobile (LEJ)

thomasgwatson merged commit 0512d0d into dev Feb 21, 2025
1 of 3 checks passed

thomasgwatson deleted the 254-my-context+user-settings-mobile branch February 21, 2025 23:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

'my' context and user settings in mobile #256

'my' context and user settings in mobile #256

thomasgwatson commented Feb 12, 2025

thomasgwatson commented Feb 12, 2025

lorenjohnson Feb 12, 2025

lorenjohnson commented Feb 12, 2025

thomasgwatson commented Feb 12, 2025

thomasgwatson Feb 13, 2025

thomasgwatson Feb 13, 2025

thomasgwatson Feb 13, 2025

lorenjohnson Feb 18, 2025

thomasgwatson Feb 13, 2025

thomasgwatson commented Feb 13, 2025

thomasgwatson commented Feb 13, 2025

thomasgwatson commented Feb 13, 2025

tibetsprague commented Feb 13, 2025

lorenjohnson commented Feb 13, 2025

sentry-io bot commented Feb 22, 2025

'my' context and user settings in mobile #256

'my' context and user settings in mobile #256

Conversation

thomasgwatson commented Feb 12, 2025

thomasgwatson commented Feb 12, 2025

lorenjohnson Feb 12, 2025

Choose a reason for hiding this comment

lorenjohnson commented Feb 12, 2025

thomasgwatson commented Feb 12, 2025

thomasgwatson Feb 13, 2025

Choose a reason for hiding this comment

thomasgwatson Feb 13, 2025

Choose a reason for hiding this comment

thomasgwatson Feb 13, 2025

Choose a reason for hiding this comment

lorenjohnson Feb 18, 2025

Choose a reason for hiding this comment

thomasgwatson Feb 13, 2025

Choose a reason for hiding this comment

thomasgwatson commented Feb 13, 2025

thomasgwatson commented Feb 13, 2025

thomasgwatson commented Feb 13, 2025

tibetsprague commented Feb 13, 2025

lorenjohnson commented Feb 13, 2025

sentry-io bot commented Feb 22, 2025

Suspect Issues