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

Completed/ongoing story strings should be separate text views rather than a combined string #3843

Closed
BenHenning opened this issue Sep 25, 2021 · 14 comments · Fixed by #5151
Closed

Completed/ongoing story strings should be separate text views rather than a combined string #3843

BenHenning opened this issue Sep 25, 2021 · 14 comments · Fixed by #5151
Labels
bug End user-perceivable behaviors which are not desirable. 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). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. 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

@BenHenning
Copy link
Member

After #3795, NavigationDrawerHeaderViewModel combines two strings (for ongoing & completed stories) into a single one. String concatenation should never be used for display purposes since it can be wrong in LTR/RTL situations. Instead, there should be separate text views or a single string to represent the text.
@BenHenning BenHenning added this to the Beta MR2 milestone Jun 11, 2022
@Broppia Broppia added issue_type_infrastructure Impact: Low Low perceived user impact (e.g. edge cases). labels Jul 7, 2022
@BenHenning BenHenning added 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. issue_user_developer labels Sep 15, 2022
@BenHenning BenHenning removed this from the Beta MR2 milestone Sep 16, 2022
@seanlip seanlip added bug End user-perceivable behaviors which are not desirable. good first issue This item is good for new contributors to make their pull request. and removed issue_type_infrastructure labels Mar 28, 2023
@jkugsiya
Copy link
Contributor

jkugsiya commented May 7, 2023

Hi, can I work on this issue, I have worked in this repo before, but its been long time, so I want to start again to contribute, so this will be a good re-starting point for me.

@MohitGupta121
Copy link
Member

Hi @jkugsiya , can you give a brief description of how you are tackling this? What files have you changed, what is the actual cause of the issue?

Feel free to post your approach here and if it looks like it is in the right direction, we can assign this to you. Thanks!

@jkugsiya
Copy link
Contributor

jkugsiya commented May 8, 2023

Hi @jkugsiya , can you give a brief description of how you are tackling this? What files have you changed, what is the actual cause of the issue?

Feel free to post your approach here and if it looks like it is in the right direction, we can assign this to you. Thanks!

Hi @MohitGupta121, the way I want to tackle this is to create two separate views to display the text, I would like to have assistance in approaching the same.

@MohitGupta121
Copy link
Member

Okay Thanks @jkugsiya Please go ahead.
Feel free to ask any things. Thanks!

@jkugsiya
Copy link
Contributor

Hi @MohitGupta121, where can I ask questions regarding this as I am starting again and want to re-gain the concepts. Or can you please brief me through this as what to be implemented.

Thanks and apologies for this much delay.

@MohitGupta121
Copy link
Member

@jkugsiya Totally fine, You can ask your queries in the Github-discussion Tab of oppia-android.

@adhiamboperes
Copy link
Collaborator

Unassigning @jkugsiya due to inactivity

@adhiamboperes adhiamboperes mentioned this issue Jul 27, 2023
Open
@adhiamboperes adhiamboperes added the Work: Low Solution is clear and broken into good-first-issue-sized chunks. label Jul 27, 2023
@dev-divyansh
Copy link

@adhiamboperes I would like to work on this issue

@adhiamboperes
Copy link
Collaborator

@dev-divyansh Feel free to post your approach here and if it looks good, we will assign this to you. Thanks!

@dev-divyansh
Copy link

@adhiamboperes Sorry , for the late reply was busy with other stuff.
I would separate the two strings and use separate views to display them.

@ayush290104
Copy link

@adhiamboperes on checking the code for welcome viewmodel and its layout what i saw is that there are already two seperate textviews for both the texts welcome and name.
Can you guide.
Screenshot (34)

@dev-divyansh
Copy link

@adhiamboperes what u say ??

@adhiamboperes
Copy link
Collaborator

@adhiamboperes what u say ??

Feel free to put up a PR showing your proposed solution.

@adhiamboperes
Copy link
Collaborator

@adhiamboperes on checking the code for welcome viewmodel and its layout what i saw is that there are already two seperate textviews for both the texts welcome and name. Can you guide. Screenshot (34)

This issue requests changes in NavigationDrawerHeaderViewModel

XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Sep 13, 2023
@XichengSpencer
Merge branch 'oppia:develop' into oppia#3843
31b6f53
@XichengSpencer XichengSpencer mentioned this issue Sep 13, 2023
Merged
6 tasks
adhiamboperes added a commit to XichengSpencer/oppia-android that referenced this issue Sep 19, 2023
@adhiamboperes
Merge branch 'develop' into oppia#3843
3406c78
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Sep 19, 2023
@XichengSpencer
Merge remote-tracking branch 'origin/oppia#3843' into oppia#3843
b1bfe10
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Sep 22, 2023
@XichengSpencer
Merge branch 'develop' into oppia#3843
8d9cf96
adhiamboperes added a commit that referenced this issue Sep 22, 2023
@XichengSpencer @adhiamboperes
Fix #3843 Split Concatenated TextView (#5151)
cfce93c 
## Explanation
Fix #3843 Instead of using 1 textview to display a concatenated string,
now have 1 textview for story progress, 1 for bar separator, and 1 for
topic progress.

## 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

|LTR|RTL|
|---|---|

|![Screenshot_20230913_170309](https://github.com/oppia/oppia-android/assets/74568012/5d10ee41-41cb-46f1-b37d-b368e247c108)|![Screenshot_20230913_170148](https://github.com/oppia/oppia-android/assets/74568012/d5ee32ce-1899-4e5c-825c-3070f5e132ad)|

|![Screenshot_20230913_165312](https://github.com/oppia/oppia-android/assets/74568012/4435074e-41b3-4b0b-9dd1-7cfc1b3bfd72)|![Screenshot_20230913_170218](https://github.com/oppia/oppia-android/assets/74568012/476060ea-15b8-4636-afa0-89fdb130d05b)|




## Tests
![Screenshot 2023-09-15
101749](https://github.com/oppia/oppia-android/assets/74568012/2ee79906-22e8-42a0-b3b6-b71676b50f92)

---------

Co-authored-by: Adhiambo Peres <[email protected]>
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Nov 15, 2023
@XichengSpencer
Merge branch 'develop' of https://github.com/oppia/oppia-android into o…
1cbe96d 
…ppia#3843
@XichengSpencer XichengSpencer mentioned this issue Nov 15, 2023
Merged
6 tasks
adhiamboperes added a commit to XichengSpencer/oppia-android that referenced this issue Dec 4, 2023
@adhiamboperes
Merge branch 'develop' into oppia#3843
582f69d
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Dec 7, 2023
@XichengSpencer
Merge remote-tracking branch 'origin/oppia#3843' into oppia#3843
6824346
adhiamboperes added a commit to XichengSpencer/oppia-android that referenced this issue Dec 7, 2023
@adhiamboperes
Merge branch 'develop' into oppia#3843
ebe5a28
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Dec 19, 2023
@XichengSpencer
Merge branch 'develop' of https://github.com/oppia/oppia-android into o…
4802202 
…ppia#3843
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Dec 19, 2023
@XichengSpencer
Merge remote-tracking branch 'origin/oppia#3843' into oppia#3843
e40a900
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Jan 2, 2024
@XichengSpencer
Merge branch 'oppia:develop' into oppia#3843
c362a55
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Jan 8, 2024
@XichengSpencer
Merge branch 'develop' of https://github.com/oppia/oppia-android into o…
698a0d1 
…ppia#3843
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Jan 8, 2024
@XichengSpencer
Merge remote-tracking branch 'origin/oppia#3843' into oppia#3843
783432a
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Jan 17, 2024
@XichengSpencer
Merge branch 'develop' of https://github.com/oppia/oppia-android into o…
a489412 
…ppia#3843
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Feb 23, 2024
@XichengSpencer
Merge branch 'develop' of https://github.com/oppia/oppia-android into o…
22040e2 
…ppia#3843
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Mar 4, 2024
@XichengSpencer
Merge branch 'develop' into oppia#3843
3094158
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Mar 26, 2024
@XichengSpencer
Merge branch 'develop' of https://github.com/oppia/oppia-android into o…
e1b26d6 
…ppia#3843
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Mar 26, 2024
@XichengSpencer
Merge remote-tracking branch 'origin/oppia#3843' into oppia#3843
f29e82d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug End user-perceivable behaviors which are not desirable. 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). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. 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.
Projects
[Team] Core Learner and Mastery flows & UI Frontend - Android
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #3843 Split Concatenated TextView XichengSpencer/oppia-android
9 participants
@seanlip @BenHenning @jkugsiya @adhiamboperes @MohitGupta121 @aayushimathur6 @dev-divyansh @Broppia @ayush290104