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

Hi-fi Resume Lesson Tablet #3691

Closed
MaskedCarrot opened this issue Aug 16, 2021 · 13 comments · Fixed by #5175
Closed

Hi-fi Resume Lesson Tablet #3691

MaskedCarrot opened this issue Aug 16, 2021 · 13 comments · Fixed by #5175
Assignees
Labels
enhancement End user-perceivable enhancements. 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). Work: Low Solution is clear and broken into good-first-issue-sized chunks. Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@MaskedCarrot
Copy link
Contributor

MaskedCarrot commented Aug 16, 2021

Update the tablet UI for resume lesson.

Mock for mobile portrait: https://xd.adobe.com/view/3dca36c2-5115-419c-b25e-0f10526b077c-6899/screen/1c7d4ec6-a5f1-4462-886d-e47c1457b69e/

@Broppia Broppia added issue_type_bug Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). labels Jun 13, 2022
@BenHenning BenHenning added issue_type_enhancement Impact: Low Low perceived user impact (e.g. edge cases). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Z-ibt Temporary label for Ben to keep track of issues he's triaged. and removed issue_type_bug Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). labels Sep 15, 2022
@seanlip seanlip added enhancement End user-perceivable enhancements. and removed issue_user_learner labels Mar 29, 2023
@adhiamboperes adhiamboperes added the Work: Low Solution is clear and broken into good-first-issue-sized chunks. label Aug 10, 2023
@adhiamboperes adhiamboperes changed the title Hi-fi Resume Lesson Tablet [Blocked on #3490] Hi-fi Resume Lesson Tablet Aug 10, 2023
@theMr17
Copy link
Collaborator

theMr17 commented Oct 1, 2023

Hi @adhiamboperes, I would like to work on this issue. My approach would be to add a layout resume_lesson_fragment.xml for sw600dp-port qualifiers and create the screen according to the UI attached with the issue.

@adhiamboperes
Copy link
Collaborator

Hi @adhiamboperes, I would like to work on this issue. My approach would be to add a layout resume_lesson_fragment.xml for sw600dp-port qualifiers and create the screen according to the UI attached with the issue.

Can you please confirm that this file doesn't exist?

@adhiamboperes adhiamboperes added the good first issue This item is good for new contributors to make their pull request. label Oct 1, 2023
@theMr17
Copy link
Collaborator

theMr17 commented Oct 1, 2023

No, the file does not exist at present.

image

@MohitGupta121
Copy link
Member

MohitGupta121 commented Oct 1, 2023

@adhiamboperes But I think we need design for Resume Lesson specifically for tablet. Is that exist?

@adhiamboperes
Copy link
Collaborator

@adhiamboperes But I think we need design for Resume Lesson specifically for tablet. Is that exist?

@MohitGupta121, for this screen I think it's fine if it looks like the mobile portrait, as in the linked mock. Cc @seanlip

@seanlip
Copy link
Member

seanlip commented Oct 1, 2023

Here are the tablet mocks I'm aware of. Maybe see if this one is included there? https://xd.adobe.com/view/d405de00-a871-4f0f-73a0-f8acef30349b-a234/grid

@theMr17
Copy link
Collaborator

theMr17 commented Oct 2, 2023

@seanlip I had a look through all the tablet mocks, but this one is not available there.

@seanlip
Copy link
Member

seanlip commented Oct 3, 2023

@the-mr17 Thanks for checking. Are there any mocks that are similar or that cover the same screen (but perhaps in a different context)? The link I sent you is all the mocks we have, so we might need to reason by analogy if the exact mock isn't provided there.

@adhiamboperes
Copy link
Collaborator

adhiamboperes commented Oct 3, 2023

#5116 shows how this screen looks on different devices. I don't forsee any issues with making the tablet portrait look like mobile portrait(mocks linked in issue description)

@MohitGupta121
Copy link
Member

MohitGupta121 commented Oct 3, 2023

@adhiamboperes adhiamboperes removed the Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. label Oct 3, 2023
@theMr17
Copy link
Collaborator

theMr17 commented Oct 3, 2023

Thanks! I am submitting a PR after implementing the screen based on the available mocks.

@theMr17
Copy link
Collaborator

theMr17 commented Oct 3, 2023

I have added some extra horizontal margin to the texts. I don't think anything else needs to changed. Please let me know your thoughts on this.

Before After

@seanlip
Copy link
Member

seanlip commented Oct 3, 2023

Thanks, I don't have any concerns here.

adhiamboperes added a commit that referenced this issue Oct 6, 2023
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

Fixes #3691

Adds layout file for tablet portrait mode for resume lesson screen.

## 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: ...".)
- [x] 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

|Before|After|
|--|--|
|<img
src="https://github.com/oppia/oppia-android/assets/84731134/00eb5bd2-eb6c-4c83-8f89-9b7f1f47290d"
width="400"/>|<img
src="https://github.com/oppia/oppia-android/assets/84731134/170808c4-0a5b-45df-b2a4-7bd312216010"
width="400"/>|
|<img
src="https://github.com/oppia/oppia-android/assets/84731134/f8c7cfd2-fff4-42e4-b1e8-c1f021a6c730"
width="400"/>|<img
src="https://github.com/oppia/oppia-android/assets/84731134/078b9960-53f4-4bb6-805d-aba62ec9f7dd"
width="400"/>|

---------

Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement End user-perceivable enhancements. 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). Work: Low Solution is clear and broken into good-first-issue-sized chunks. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Development

Successfully merging a pull request may close this issue.

7 participants