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

Fix #2266: [RTL] High-fi Admin and User PinVerification Screen #3684

Merged
merged 168 commits into from
Oct 6, 2021

Conversation

veena14cs
Copy link
Contributor

@veena14cs veena14cs commented Aug 15, 2021

Explanation

This PR fixes #2266 . This PR implements new design for Admin and User Pin Verification
Older implementation of Pin Verification UI was using PinView library which was not supporting RTL. To fix this we removed this library and created new UI as per the new mocks.

Old Screenshot LTR and RTL

Screenshot_1629793897........Screenshot_1629793938
Screenshot_1629793905........Screenshot_1629793945

New Screenshot LTR and RTL

Screenshot_1633506133...Screenshot_1633506065

Screenshot_1633506118.....Screenshot_1633506083

Tablet

Screenshot_1633504178.....Screenshot_1633504220

Screenshot_1633504169.....Screenshot_1633504231

Mock links

Mobile Portrait : https://xd.adobe.com/view/3dca36c2-5115-419c-b25e-0f10526b077c-6899/screen/4d274b3e-c27b-438a-8c74-c0ff16134080/
Mobile Landscape : https://xd.adobe.com/view/ee9e607b-dbd6-4372-48dc-b687d32af3da-98af/screen/327723da-a1e8-40e4-aace-9c30d663b20b/
Tablet Portrait : https://xd.adobe.com/view/d405de00-a871-4f0f-73a0-f8acef30349b-a234/screen/8de829a5-fc1c-4083-b510-44c1387ed9c2/
Tablet Landscape : https://xd.adobe.com/view/d405de00-a871-4f0f-73a0-f8acef30349b-a234/screen/0648610b-7d70-403c-a42e-a206be75e85f/

Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
  • The PR follows the style guide.
  • The PR does not contain any unnecessary auto-generated code from Android Studio.
  • The PR is made from a branch that's not called "develop".
  • The PR is made from a branch that is up-to-date with "develop".
  • The PR's branch is based on "develop" and not on any other branch.
  • The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

@veena14cs veena14cs requested a review from rt4914 October 5, 2021 20:40
@veena14cs veena14cs assigned rt4914 and BenHenning and unassigned veena14cs Oct 5, 2021
@veena14cs
Copy link
Contributor Author

@veena14cs The UI does not look correct.

  1. The label Administrator's 5 Digit PIN UI is different from that of admin_auth_activity.
  2. In landscape mode the UI does not look in centre.
  3. Toolbar shadow is not correct.
    Screenshot_1633438399

Also, suggest updating the screenshots in PR Description

@rt4914 I have addressed and updated the changes

  1. Changed the UI keeping it consistent with admin_auth_activity.
  2. Centered UI at the center in landscape mode.
  3. Corrected Toolbar shadow in Tablet mode.

Updated screenshots in PR description. PTAL Thanks.

@veena14cs veena14cs removed their assignment Oct 5, 2021
Copy link
Sponsor Member

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

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

Thanks @veena14cs. This LGTM; deferring to Rajat for final approval.

@rt4914 please re-add me if anything changes from the build or testing side.

@BenHenning BenHenning removed their assignment Oct 5, 2021
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@rt4914 rt4914 assigned veena14cs and unassigned rt4914 Oct 6, 2021
@oppiabot oppiabot bot added the PR: LGTM label Oct 6, 2021
@veena14cs veena14cs merged commit 5c3eaf3 into oppia:develop Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: LGTM Type: Task A single task of work corresponding to a greater milestone. Generally corresponds to a single PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RTL] High-fi AdminPinVerification [BLOCKED]
5 participants