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

Use new result API #5875

Merged
merged 15 commits into from
Oct 23, 2024
Merged

Conversation

parneet-guraya
Copy link
Contributor

@parneet-guraya parneet-guraya commented Oct 19, 2024

Fixes #5849

What changes did you make and why?

  • Migrate Activities that are launched with expectancy of getting result back, to new result api.

For ease of review, these are the functionalities that are affected and have been tested.

  • File picking -> Camera, Gallery, Custom; from every place one or more of these options can be used have been refactored.
  • getting result back from ImageFragment launched from CustomSelectorActivity
  • getting result of voice input.
  • result from image editing from EditActivity

Tests performed (required)
betaDebug & prodDebug tested on Oneplust 9RT 5G , Android 14

@parneet-guraya parneet-guraya marked this pull request as ready for review October 20, 2024 23:56
Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

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

I have tested the features you mentioned, they seem to be working great on Pixel 9 too. :-)
I will try some uploads tomorrow.

Meanwhile, I noticed a few places missing a comma after , would you mind fixing them?

Thanks a lot!

@parneet-guraya
Copy link
Contributor Author

Meanwhile, I noticed a few places missing a comma after , would you mind fixing them?

I'm not quite sure what you meant by that, are you talking about adding a space just after the comma?

Also, I know it needs reformatting, I tried it but it also reformats the code which isn't part of the my change (maybe somebody missed reformatting previously) that's why I manually did reformatting and might have missed some places. However, if you say I can reformat all the affected files just know that it will also include the change which I didn't make.

@parneet-guraya
Copy link
Contributor Author

Also what do we use for updating branches? I saw you used merge, but I was using rebase until now. Do we follow any standard?

@nicolas-raoul
Copy link
Member

nicolas-raoul commented Oct 22, 2024

I tested more today, it is working great, I will merge once the commas are fixed. :-)

adding a space just after the comma

Yes, for instance initiateCameraUpload(activity,resultLauncher); should be initiateCameraUpload(activity, resultLauncher);

Merge and rebase are fine, the whole pull request is eventually squashed into a single commit at the end anyway. :-)

Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
… activity, hence removed

Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
Signed-off-by: parneet-guraya <[email protected]>
@parneet-guraya
Copy link
Contributor Author

Added requested changes b68256d

Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

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

Great work, thanks!

@nicolas-raoul nicolas-raoul merged commit 1e7aaba into commons-app:main Oct 23, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants