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.

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

Tab navigation custom behaviours #932

Open
wants to merge 18 commits into
base: dev
Choose a base branch
from

Conversation

boscojwho
Copy link
Contributor

@boscojwho boscojwho commented Feb 17, 2024

Pull Request Information

About this Pull Request

Allows users to customize tab bar actions behaviour:

  • System: Pop to root, then scroll to top (and pop back to sidebar, if available).
  • Dismiss: Go back to previous page until root page, then scroll to top (and pop back to sidebar, if available).
  • Dismiss after Scroll: Always scroll-to-top before going back to previous page (and pop back to sidebar, if available).

Appreciate any input on behaviour naming and explanation text.

Screenshots and Videos

@boscojwho boscojwho requested a review from a team as a code owner February 17, 2024 00:54
@boscojwho boscojwho requested review from JakeShirley and mormaer and removed request for a team February 17, 2024 00:54
@Sjmarf
Copy link
Contributor

Sjmarf commented Feb 17, 2024

I think the wording here could be improved somewhat.

Would an inline list picker give us more space (the one with the checkmarks)? Maybe it could be titled "Tap tab bar icon to..." or "When I tap the tab bar icon..." and have the following options:

  • "Do nothing"
  • "Scroll to top"
  • "Return to previous page"
  • "Return to first page"

@boscojwho
Copy link
Contributor Author

I think the wording here could be improved somewhat.

Would an inline list picker give us more space (the one with the checkmarks)? Maybe it could be titled "Tap tab bar icon to..." or "When I tap the tab bar icon..." and have the following options:

  • "Do nothing"
  • "Scroll to top"
  • "Return to previous page"
  • "Return to first page"

Yea, that could work better, will tweak and update

@boscojwho
Copy link
Contributor Author

@Sjmarf
Copy link
Contributor

Sjmarf commented Feb 19, 2024

Looks good! I think it might look a little better in Title Case, without the trailing ellipses?

@EricBAndrews
Copy link
Member

We might want to move all the tab bar stuff into its own section of Settings--with this PR we're going to have:

  • Account switcher behavior in Profile
  • Tab bar appearance in Appearance
  • Tab tap behavior in General

Certainly not a blocker to this PR though

@Sjmarf
Copy link
Contributor

Sjmarf commented Feb 20, 2024

That could work.

What about the other categories? There are other cases where we will have a similar split-up grouping in future. An example would be when we add swipe action customisation, which in our current layout would go inside of a new "Gestures" page. That splits them up from the interaction bar customisation, which feels slightly off to me. And what if we then add ellipsis menu customisation too? Where would that go? It would be strange to put that in Appearance.

One solution go the proposed tab-bar settings route for the other categories too, and simply put all of the navigation links that are currently under Appearance under the root view. Then, gestures and ellipsis menu customisation could go in the same menu as appearance settings.

@boscojwho
Copy link
Contributor Author

Looks good! I think it might look a little better in Title Case, without the trailing ellipses?

Both work, I chose the ellipses without title case to indicate that there's more to the action than the literal name (i.e. nudge users to read the footer text lol)

@boscojwho
Copy link
Contributor Author

We might want to move all the tab bar stuff into its own section of Settings--with this PR we're going to have:

  • Account switcher behavior in Profile
  • Tab bar appearance in Appearance
  • Tab tap behavior in General

Certainly not a blocker to this PR though

Yea, tab bar stuff definitely needs its own little page soon with the hide tab bar on scroll feature (going to wait for v2.0), and also thinking about long press to scroll to top (also going to wait for v2.0).

@EricBAndrews
Copy link
Member

EricBAndrews commented Feb 20, 2024

Re: menu names--these behaviors are super complex to describe in a single list entry. Ellipsis + footer works well enough for me as a pro tem solution, but longer term we might want to look into using animations to describe the behavior the way we do with the quick switcher

@boscojwho
Copy link
Contributor Author

Re: menu names--these behaviors are super complex to describe in a single list entry. Ellipsis + footer works well enough for me as a pro tem solution, but longer term we might want to look into using animations to describe the behavior the way we do with the quick switcher

Yea, eventually animations, figured I'd get this in without them for now because of time.

Copy link
Member

@EricBAndrews EricBAndrews left a comment

Choose a reason for hiding this comment

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

Nice! Works pretty much perfectly. I found a couple minor defects while testing:

  • Scroll to Top does not behave properly in Community view
  • Do Nothing still scrolls to top in Feeds view

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants