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

Refactor: Intent creation with flags across codebase for back stack management #17944

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Akshit517
Copy link

Description

This draft PR introduces utility functions to standardize and simplify Intent creation across some parts of the codebase. The goal is to ensure consistency in back stack management and reduce redundant code. Seeking feedback on the current implementation.

Issue

Approach

It wraps intent creations with utility functions for better management of flags.

How Has This Been Tested?

  • Google Pixel 7 pro

Checklist

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

Copy link

welcome bot commented Feb 9, 2025

First PR! 🚀 We sincerely appreciate that you have taken the time to propose a change to AnkiDroid! Please have patience with us as we are all volunteers - we will get to this as soon as possible.

@lukstbit
Copy link
Member

lukstbit commented Feb 9, 2025

First of all, thank you for contributing.

To be honest I don't see the point of the new utilities. I think the original code with the Intent creation/applying flags reads better and it's more expressive. There's almost no line count improvement as well. Starting an activity should be though at the moment when that call is introduced, forcing an utility for this sounds like a good way to introduce more bugs.

Also, we are looking into extracting more screens to fragments and with this I hope we reduce the ridiculous amount of activities we have in the app and so remove any need for these utilities.

IMO

@Akshit517
Copy link
Author

Akshit517 commented Feb 9, 2025

@lukstbit ,Thanks for your feedback!

I understand your concerns about readability and potential bugs. That said, as noted in this reply, Intent creation and flag usage are quite inconsistent across the codebase, and this seems to be an area that’s both tricky to get right and not widely understood.

The goal of these utilities isn’t just to reduce line count but to improve consistency and correctness. I think standardizing this would be helpful.

@Akshit517
Copy link
Author

Akshit517 commented Feb 12, 2025

@lukstbit @mikehardy Sorry for the ping, just wanted you to check in on my reply to your feedback. Thanks again!

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

Successfully merging this pull request may close these issues.

2 participants