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 248 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.

avazirna and others added 12 commits May 6, 2024 13:53
…hanges

Improve audio recording configuration
User shown a reminder tile on Connect Jobs and app home pages for a week after registration, prompting them to perform verification.
After a week, user is forced to perform secondary phone verification in order to unlock ConnectId.
Deleting all Connect DB data when user severs link to their ConnectId account.
Added registration date to ConnectUserRecord
Showing Connect apps in front login page, user can login to them directly.
Arrow button in jobs lists for jobs in progress now goes directly to learn/deliver app.
Additional button now shown to take user to current job info page.
Daily progress bar added to app home page when in delivery app (and job not finished yet).
"View Job Status" button added to app home page for Connect apps.
Backing up/logging out from app home page always goes back to login screen, i.e. locks ConnectID.
…ready forgot their PIN by that point.

Swapped Sync and Connect buttons on app home page (better visual alignment on UI).
@OrangeAndGreen
Copy link
Contributor Author

@damagatchi Retest this please

Dave Viggiano added 2 commits May 21, 2024 11:52
… Navigation workflow).

Restored default nav host for ConnectActivity.
Finishing ConnectActivity when launching learn/deliver app.
Disabled buttons to launch learn/deliver app when viewing job info from inside an app.
@OrangeAndGreen
Copy link
Contributor Author

@damagatchi Retest this please

@OrangeAndGreen
Copy link
Contributor Author

@damagatchi Retest this please

@OrangeAndGreen
Copy link
Contributor Author

@damagatchi Retest this please

Dave Viggiano and others added 3 commits May 23, 2024 13:10
…gs in to app.

Restored app launch buttons by default in job info pages.
Not clearing active job whenever jobs list shows.
…jump directly to learn/delivery progress.

Deleted ConnectJobInfoActivity (no longer necessary).
Fixing single-activity with Navigation component and multiple entry points
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

3 participants