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.

Sign up for GitHub

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

Jump to bottom

Optimise code in profile_chooser_add_view.xml #1956

Closed
MohamedMedhat1998 opened this issue Oct 8, 2020 · 10 comments · Fixed by #4844
Closed

Optimise code in profile_chooser_add_view.xml #1956

MohamedMedhat1998 opened this issue Oct 8, 2020 · 10 comments · Fixed by #4844
Assignees
@Uticodes
Labels
good first issue This item is good for new contributors to make their pull request. Impact: Low Low perceived user impact (e.g. edge cases). Priority: Nice-to-have This work item is nice to have for its milestone. Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@MohamedMedhat1998
Copy link
Contributor

This issue is tracking the optimization work of profile_chooser_add_view.xml

  1. Make sure that the layout uses ConstraintLayout and there is minimal nesting of views. If this view already uses ConstraintLayout then it's fine.

  2. Make sure that all TextViews use the style from styles.xml file like Heading1, TextFieldLabel, Body, etc. whichever suits best.

While making the above changes the UI should not get affected and the end result should still be the same as before (or an optimized version of UI).

If there are multiple files with the same name, update them all.

References

Mobile Portrait: https://xd.adobe.com/view/e8aa4198-3940-47f9-514a-f41cc54457f6-9e9b/grid/
Mobile Landscape: https://xd.adobe.com/view/ee9e607b-dbd6-4372-48dc-b687d32af3da-98af/grid/
Tablet: https://xd.adobe.com/view/d405de00-a871-4f0f-73a0-f8acef30349b-a234/grid/
Wiki: https://github.com/oppia/oppia-android/wiki/Working-on-UI
@MohamedMedhat1998 MohamedMedhat1998 added Type: Improvement Priority: Nice-to-have This work item is nice to have for its milestone. good first issue This item is good for new contributors to make their pull request. Hacktoberfest This is a suggested Hacktoberfest issue. labels Oct 8, 2020
@MohamedMedhat1998 MohamedMedhat1998 added this to the Backlog milestone Oct 8, 2020
@Arjupta
Copy link
Contributor

Arjupta commented Oct 11, 2020

I will work on this .

@MohamedMedhat1998
Copy link
Contributor Author

I will work on this .

@Arjupta Before we assign you this one, can you finalize #1042 first, please?
If you are facing difficulties with #1042 we can remove your assignment there and assign you this one.
What would you prefer?

@Arjupta
Copy link
Contributor

Arjupta commented Oct 11, 2020

I am working on the #1042, but as @rt4914 suggested to look upon the rest Slop issues as that issue will take time , so I started working on this too. I will take the #1042 if have to choose between one.

@MohamedMedhat1998
Copy link
Contributor Author

MohamedMedhat1998 commented Oct 11, 2020 •
edited
Loading

Ok no issues, you can work on both. I'll assign this one to you too.

Arjupta added a commit to Arjupta/oppia-android that referenced this issue Oct 12, 2020
@Arjupta
Fixes oppia#1956 :Optimise profile_chooser_add_view.xml
9ab4579
@Arjupta Arjupta mentioned this issue Oct 12, 2020
Closed
8 tasks
@BenHenning BenHenning removed the Hacktoberfest This is a suggested Hacktoberfest issue. label Dec 3, 2020
@arinarora18
Copy link

May I work on this?

@MohamedMedhat1998
Copy link
Contributor Author

May I work on this?

@rt4914 @Sarthak2601 @aggarwalpulkit596

@rt4914
Copy link
Contributor

rt4914 commented Aug 23, 2021

@arinarora18 Suggest working on #3556 first and then work on this issue.

@Broppia Broppia added the Impact: Low Low perceived user impact (e.g. edge cases). label Jul 14, 2022
@Akshatkamboj14 Akshatkamboj14 self-assigned this Aug 9, 2022
@Akshatkamboj14 Akshatkamboj14 mentioned this issue Sep 9, 2022
Closed
6 tasks
@BenHenning BenHenning added Z-ibt Temporary label for Ben to keep track of issues he's triaged. issue_user_developer labels Sep 15, 2022
@BenHenning BenHenning removed this from the Backlog milestone Sep 16, 2022
@rt4914 rt4914 moved this from Needs Triage to In Progress: Being implemented in [Team] Core Learner and Mastery flows & UI Frontend - Android Sep 20, 2022
@Akshatkamboj14 Akshatkamboj14 removed their assignment Oct 4, 2022
@Uticodes
Copy link
Contributor

Uticodes commented Dec 6, 2022

Hi @MohitGupta121,
I'd like to work on this issue.
Thank you 🙏

@MohitGupta121
Copy link
Member

@Uticodes assigned go ahead.
If you have any query feel free to ask me.

@Uticodes
Copy link
Contributor

Uticodes commented Dec 6, 2022 via email

@Uticodes Uticodes mentioned this issue Dec 11, 2022
Closed
6 tasks
@aayushimathur6 aayushimathur6 removed their assignment Jan 10, 2023
@Uticodes Uticodes mentioned this issue Jan 15, 2023
Merged
6 tasks
rt4914 pushed a commit that referenced this issue Jan 30, 2023
@Uticodes
Fixed #1956: Optimise code in profile chooser add view (#4844)
2dfad61 
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fixed #1956: Optimise code in profile chooser add view

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [ ] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-(A11y)-Guide))
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

| Screenshots Before |  After
|:-------------------------:|:-------------------------:|
| Mobile Portrait |

|![mobile_portrait_before](https://user-images.githubusercontent.com/43546652/206875860-745fe9bb-d9e6-4951-961f-89115567c778.png)
|
![mobile_portrait_after](https://user-images.githubusercontent.com/43546652/206875863-14733b63-8c09-4626-b89d-ebb55dfe6ba7.png)
| Mobile Landscape |

|![mobile_land_before](https://user-images.githubusercontent.com/43546652/206875865-b14b6106-9d8e-4890-ac67-038c23b2b2e9.png)
|
![mobile_land_after](https://user-images.githubusercontent.com/43546652/206876484-0115c672-ddf4-40f5-980d-c0119e318a38.png)
| Tap Portrait |
|
![tap_portrait_before](https://user-images.githubusercontent.com/43546652/206875870-4051acbe-5c46-47c8-8d79-cfc4cd4f4a38.png)
|
![tap_portrait_after](https://user-images.githubusercontent.com/43546652/206875871-7f70d5e6-7a8a-4bfa-bc96-cc49ee7a4539.png)
| Tap Landscape |

|![tap_land2_before](https://user-images.githubusercontent.com/43546652/206876590-0386cc8e-a3d5-43f7-bf11-4ea0ad48fb48.png)
|
![tap_land2_after](https://user-images.githubusercontent.com/43546652/206875868-58d33fcb-6c67-4300-b887-dd8dd16ca586.png)
| Tap LTR Portrait |
|
![tap_lrt_portrait_before](https://user-images.githubusercontent.com/43546652/206875874-4b0878a1-3d78-4dfc-8c90-473d1cefe7f7.png)
|
![tap_ltr_portrait_after](https://user-images.githubusercontent.com/43546652/206875876-fb42684f-f8b2-41bc-83a7-ea6d58322cb6.png)
| Tap LTR Landscape |

|![tap_ltr_land_before](https://user-images.githubusercontent.com/43546652/206875877-40949643-ac03-4743-a0ae-5a266a67490a.png)
|
![tap_ltr_land_after](https://user-images.githubusercontent.com/43546652/206875879-bd9cc1f6-5f4c-48b8-a345-3781f82bab67.png)
| Mobile LTR Portrait |

|![mobile_ltr_before](https://user-images.githubusercontent.com/43546652/206875880-717ab192-ba03-4b88-821d-21e1500bf301.png)
|
![mobile_ltr_portrait_after](https://user-images.githubusercontent.com/43546652/206875881-a9fc3334-7960-4401-a681-7e4472fd2f17.png)
| Mobile LTR Landscape |

|![mobile_ltr_land_before](https://user-images.githubusercontent.com/43546652/206875882-c8033397-620d-411f-b3ec-353959320355.png)
|
![mobile_ltr_land_after](https://user-images.githubusercontent.com/43546652/206875883-36b9b7bd-ea85-41c9-b74f-e0cf85e6baf3.png)

## Accessibility Guide 

https://user-images.githubusercontent.com/43546652/206877043-0e4221bc-b991-4b4d-8ae7-785eac6d4644.mov


## Unit Test Screenshot
<img width="852" alt="Screenshot 2022-12-09 at 7 52 10 AM"
src="https://user-images.githubusercontent.com/43546652/206876749-bda902b6-21b1-4102-8ace-f0fe41bf9877.png">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Uticodes Uticodes

Labels
good first issue This item is good for new contributors to make their pull request. Impact: Low Low perceived user impact (e.g. edge cases). Priority: Nice-to-have This work item is nice to have for its milestone. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Projects
[Team] Core Learner and Mastery flows & UI Frontend - Android
Archived in project
Milestone
No milestone