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

Refresh the navbar with new icons and animations #1167

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

meenbeese
Copy link
Contributor

@meenbeese meenbeese commented Aug 4, 2024

This PR enhances the navbar icons and transition animations, thus improving navigation, clarity, and content.

Part of #790
Closes #911

Icons:

Now each navbar icon has two variations: outline and filled. The filled one is used when the item is selected, and the outline is used otherwise. This is done via a selector for each of the icons that manages this, so the navbar icon can be set to the selector. The old icons are not deleted and new ones are defined instead to not break other usages of the icons. This approach helps maintain consistency and ensures that the navigation experience is intuitive and visually appealing.

Before After

Animations:

There are four animations in total, which are the left and right variations of both slide-in and slide-out. These animations are used to create a smooth transition effect when navigating between different fragments in the app. The navOptionsLeftToRight is used when navigating to a fragment with a higher item ID, and navOptionsRightToLeft is used when navigating to a fragment with a lower item ID. This ensures that the transitions feel natural based on the direction of navigation.

Before:

Animations-before.webm

After:

Animations-after.webm

Tested on Pixel 8 API 34 Emulator.

@meenbeese meenbeese changed the title Refresh more icon using Material icons Refresh the navbar with new Material icons Aug 10, 2024
@meenbeese meenbeese force-pushed the more-icon branch 2 times, most recently from 9263682 to c7e3f72 Compare August 10, 2024 16:43
@meenbeese meenbeese changed the title Refresh the navbar with new Material icons Refresh the navbar with new icons and animations Aug 21, 2024
@n8fr8
Copy link
Member

n8fr8 commented Oct 7, 2024

I like a lot of this, but why did you replace the Onion connect with a key?

@meenbeese
Copy link
Contributor Author

I like a lot of this, but why did you replace the Onion connect with a key?

The onion icon was found to be confusing in #911, and the best replacement I found from Google Icons was the key. I thought it signified a "secure connection" as it relates to Tor, but I am open to changing it for sure.

@syphyr
Copy link
Contributor

syphyr commented Jan 24, 2025

This is a really nice improvement to the UI,, but I think guardianproject would rather use their tor icon instead of the key.

@syphyr
Copy link
Contributor

syphyr commented Jan 24, 2025

Do you think you could add an additional patch that replaces the key with the onion icon? This way, both icons can be reviewed.

@syphyr
Copy link
Contributor

syphyr commented Jan 28, 2025

This PR introduces a bug where the Ports do not get updated upon initial start on the MoreFragment. It just says "no ports set" when Orbot is started up. If I go into the settings and update the port, then it shows up in the MoreFragment.

It seems like the bug is caused by removing lateinit.

@n8fr8
Copy link
Member

n8fr8 commented Jan 31, 2025

@meenbeese happy to merge this if you address the bug from above and revert to the icon we have for Connect.

Thanks for all the work!

@meenbeese
Copy link
Contributor Author

@meenbeese happy to merge this if you address the bug from above and revert to the icon we have for Connect.

Thanks for all the work!

Thanks for the feedback. I fixed the bug and reverted back the connect icon.

@syphyr
Copy link
Contributor

syphyr commented Feb 1, 2025

@meenbeese happy to merge this if you address the bug from above and revert to the icon we have for Connect.
Thanks for all the work!

Thanks for the feedback. I fixed the bug and reverted back the connect icon.

On the "More" Fragment, it still says "Proxy Ports: Ports Not Set" after connecting to tor with a direct connection. I am also in Power User Mode, if that makes a difference.

@meenbeese
Copy link
Contributor Author

On the "More" Fragment, it still says "Proxy Ports: Ports Not Set" after connecting to tor with a direct connection. I am also in Power User Mode, if that makes a difference.

I actually don't know why this is still the case. I have reverted all of my changes related to the logic and fragments. Can you please test it on the master branch and see if it happens there as well? It might be a regression unrelated to this PR.

@syphyr
Copy link
Contributor

syphyr commented Feb 2, 2025

On the "More" Fragment, it still says "Proxy Ports: Ports Not Set" after connecting to tor with a direct connection. I am also in Power User Mode, if that makes a difference.

I actually don't know why this is still the case. I have reverted all of my changes related to the logic and fragments. Can you please test it on the master branch and see if it happens there as well? It might be a regression unrelated to this PR.

I just tested latest fix (with and without power user mode enabled) and the ports are still not being set after connecting to tor with a direct connection.

Screenshot_20250202

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.

Change 'connect' icon
3 participants