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

Initial development on Connect workflows #2695

Open
wants to merge 239 commits into
base: feature/connect
Choose a base branch
from

Conversation

OrangeAndGreen
Copy link
Contributor

@OrangeAndGreen OrangeAndGreen commented Aug 24, 2023

Summary

Connect navigation flow

Added Android Navigation component (and SafeArgs plugin) to the project.
Implemented Connect navigation graph.
Implemented 5 UI screens as fragments and built/populated the UIs.

Product Description

When user unlocks ConnectID on login page, button saying "Go to Connect Menu" now appears.
When clicked, the user sees the Jobs List fragment, and can begin navigating through screens with mock data.

Safety Assurance

  • [x ] If the PR is high risk, "High Risk" label is set
  • [x ] I have confidence that this PR will not introduce a regression for the reasons below
  • [x ] Do we need to enhance manual QA test coverage ? If yes, "QA Note" label is set correctly

Automated test coverage

No tests yet

Configured project to use Android Navigation component.
Created nav graph for Connect workflows.
Created 5 of the Connect pages and built UIs (some work still to do).
Copy link
Contributor

@shubham1g5 shubham1g5 left a comment

Choose a reason for hiding this comment

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

The drawable added in the PR should be added according to different Android screen densities - ldpi, hdpi, xhdpi, xxhdpi, xxxhdpi . You should be able to ask the designer for these.

@@ -606,6 +606,35 @@
<string name="connect_pictures_skip" cc:translatable="true">Skip</string>
<string name="connect_pictures_continue" cc:translatable="true">Continue</string>

<string name="connect_title" cc:translatable="true">Connect</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually I think all connect strings should have cc:translatable not set. cc:translatable is meant for CC app's translations which are contained in the app ccz bundle and can be different for each apps. But Connect translations are independent of CC app and will be standard across different CC apps.

View view = inflater.inflate(R.layout.fragment_connect_delivery_details, container, false);

TextView textView = view.findViewById(R.id.connect_delivery_title);
textView.setText(getString(R.string.connect_delivery_review_title));
Copy link
Contributor

Choose a reason for hiding this comment

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

you can set static strings in the xml itself.

public View onCreateView(LayoutInflater inflater, ViewGroup container,
Bundle savedInstanceState) {
ConnectJob job = ConnectJobIntroFragmentArgs.fromBundle(getArguments()).getJob();
getActivity().setTitle(job.getTitle());
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use a Android view model to bind the UI here instead ? That will take care of things like restoring data state on configuration changes.

return view;
}

private static class MyJobsAdapter extends RecyclerView.Adapter<RecyclerView.ViewHolder> {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably be in a separate file as JobsAdapter which can be used both for all jobs and my jobs tabs.

@OrangeAndGreen OrangeAndGreen added this to the 2.55 milestone Sep 14, 2023
Implemented UI element for circular progress bar.
Implemented database storage and first API call.
Changed all Connect strings to not specify translatable.
@shubham1g5
Copy link
Contributor

@OrangeAndGreen Wants to flag that this PR has taken the same course as earlier Connect PR and is becoming very big to manage. Not asking to change anything with the current PR but we must shift our approach on future connect work to do smaller PRs with smaller commits and try to close existing PRs proactively by requesting reviews on them and addressing feedback before taking on new work. As an example, This Android 13 PR demonstrates how to break changes in individual commits for them to be easily reviewed by commit.

Dave Viggiano added 14 commits June 18, 2024 10:05
…le-call flow.

New API call to fetch DB key from ConnectID.
Startup code to fetch DB key when missing.
Storing received DB key on registration/recovery/fetch.
Still need to add the DB key migration code.
…ter confirming primary OTP.

Improved messaging when verifying secondary OTP to only show phone number if mobile has it.
Changed text to only display last 4 digits of phone number when prompting user for OTP.
Latest attempts at migrating DB encryption.
…nd made page scrollable.

Fixed text overflow in notification tiles and made sure all are hidden by default.
Fixed a bug in biometrics that could prevent older Android users from recovering account.
@OrangeAndGreen
Copy link
Contributor Author

@damagatchi retest this please

2 similar comments
@OrangeAndGreen
Copy link
Contributor Author

@damagatchi retest this please

@OrangeAndGreen
Copy link
Contributor Author

@damagatchi retest this please

@OrangeAndGreen OrangeAndGreen removed the skip-integration-tests Skip android tests. label Jun 25, 2024
@OrangeAndGreen
Copy link
Contributor Author

@damagatchi retest this please

@OrangeAndGreen OrangeAndGreen added the skip-integration-tests Skip android tests. label Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-integration-tests Skip android tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants