Skip to content

Improve QR code flow & permissions, and update lots of assorted build config & Android API usage #32

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

Merged
merged 3 commits into from
Feb 3, 2025

Conversation

cHAuHaNz
Copy link
Contributor

@cHAuHaNz cHAuHaNz commented Jan 18, 2025

  • Improved the UI of ApplicationsList screen
  • Apps can also be launched by clicking on app from the list
  • Converted isProbablyEmulator to a function, since the value is only used once (when the app is started), we do not need to retain it in the memory by using a variable
  • Fixed - If camera permission is denied, clicking Scan QR button shows couldn't connect (Videos Attached)
  • Migrated to ActivityResultContracts instead of using deprecated startActivityForResult()
  • Migrated lib dependencies to Version Catalogs
  • Migrated build configuration from Groovy to Kotlin
Existing UI New UI
AppsDarkOriginal AppsDark
AppsLightOriginal AppsLight
AppsDialogDarkOriginal AppsDialogDark
AppsDialogLightOriginal AppsDialogLight



HttpToolkitCameraBug.webm



HttpToolkitCameraBugFixed.webm

Tested on Android 9, Android 11 & Android 15

@pimterry
Copy link
Member

Hi @cHAuHaNz. Thanks for contributing! I'm afraid I'm away right now, so I'm not going to review this immediately, but I'll take a closer look next week.

@pimterry
Copy link
Member

Hi @cHAuHaNz, can you explain more about what you mean by "Improved the UI of ApplicationsList screen"? It looks like the main change is moving the dropdown menu into a modal popup, but I'm not sure what benefit that provides.

@cHAuHaNz
Copy link
Contributor Author

Hi @pimterry, I've updated the PR to have Before/After screenshots of the UI in question. You can have a took there. Also, I have made some more improvements in the project. Do check the bullet points once again.

@pimterry
Copy link
Member

Ok, thanks for increasing the detail there. I appreciate the work you've done here, but I'm afraid I'm not sure I want to include some of these changes. I'll go through the list:

  • Improved the UI of ApplicationsList screen
    • It's subjective, but personally I think the menu, UI without lines, and toggle instead of checkboxes is clearer, so I don't want to include this
  • Apps can also be launched by clicking on app from the list
    • I think this is more likely to be confusing than helpful - I don't think people open this menu intending to open the app they select.
  • Converted isProbablyEmulator to a function, since the value is only used once (when the app is started), we do not need to retain it in the memory by using a variable
    • Saving a single boolean from memory isn't really a meaningful improvement, no Android device is that tight on memory, and while we don't currently use this later we might want to, so it's mildly better to avoid recalculating it in that case. It's not a big deal, but I don't think this is worth changing.
  • Fixed - If camera permission is denied, clicking Scan QR button shows couldn't connect (Videos Attached)
    • This is very useful! Thank you, good idea.
  • Migrated to ActivityResultContracts instead of using deprecated startActivityForResult()
    • Also very useful, thanks I've been meaning to explore that.
  • Migrated lib dependencies to Version Catalogs
    • That's a nice update, thank you
  • Migrated build configuration from Groovy to Kotlin
    • That's a nice update, thank you

Sorry, I suspect that's not what you want to hear, in future do feel free to open issues with suggestions before working on them and I'm happy to discuss in advance.

The improvements I've ticked in the list above though would genuinely be very nice to have. Could you simplify the code you've included here to just those changes, and rebase to drop the commits for the other changes? Once that's done, I'm happy to do a proper review of those changes in depth.

… couldn't connect

Migrated to `ActivityResultContracts` instead of using deprecated `startActivityForResult()``
Migrated lib dependencies to Version Catalogs
Migrated build configuration from Groovy to Kotlin
@cHAuHaNz
Copy link
Contributor Author

@pimterry Its done, feel free to let me know if you need anything else.

@pimterry pimterry merged commit 3feedb7 into httptoolkit:main Feb 3, 2025
5 of 7 checks passed
@pimterry
Copy link
Member

pimterry commented Feb 3, 2025

Thanks @cHAuHaNz! Lots of nice improvements here, great work. I'll release this as v1.5.1 shortly 😄

@pimterry pimterry changed the title Improved UI of ApplicationsList screen & other optimisations Improve QR code flow & permissions, and update lots of assorted build config & Android API usage Feb 3, 2025
@cHAuHaNz
Copy link
Contributor Author

cHAuHaNz commented Feb 3, 2025

Sure thing @pimterry! You're Welcome 😇
I hope I can contribute more to this project in future. HTTPToolkit is once of great tools I've used for traffic inspection ever.
I didn't know its open source at first, then I came across this 😅 and thought I can surely contribute to this project.

@pimterry
Copy link
Member

pimterry commented Feb 3, 2025

More contributions very welcome! Let me know if you want to discuss potential ideas. All contributors do get free Pro - if that is something you're interested in, just let me know your email (here or by emailing me at tim @ httptoolkit.com) and I'll set you up.

@cHAuHaNz
Copy link
Contributor Author

Well never say no to anything free, so I'll drop an email on the address you mentioned above. Thanks for being generous.

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.

2 participants