Fix #2581: Marquee auto restart issue #4392

KevinGitonga · 2022-06-13T07:30:19Z

Explanation

Fix #2581
This fix introduces a custom View "MarqueeView" which acts as parent view for the textview in the toolbar this allows the MarqueeView to manipulate the child TextView and allow's it to marquee through translation animation if the text is long enough. It also solves the issue which we had which of text animating over the back navigation icon by attaching the textview inside a scrollview which allows it disappear cleanly without overlapping.

Essential Checklist

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 files have their rationale included in the PR explanation.
The PR follows the style guide.
The PR does not contain any unnecessary code changes from Android Studio (reference).
The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

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)
Add a video showing the full UX flow with a screen reader enabled (see accessibility guide)
Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

Before changes demonstration

marquue_restart_issue_bf.mp4

After changes demonstration

after_demo.mp4

After changes RTL demonstration

marquee_rtl_demo.mp4

rt4914

@KevinGitonga Suggested changes.

...oppia/android/app/devoptions/markchapterscompleted/MarkChaptersCompletedFragmentPresenter.kt

...g/oppia/android/app/devoptions/markstoriescompleted/MarkStoriesCompletedFragmentPresenter.kt

...org/oppia/android/app/devoptions/marktopicscompleted/MarkTopicsCompletedFragmentPresenter.kt

app/src/main/java/org/oppia/android/app/topic/TopicFragmentPresenter.kt

…into fix-marquee-auto-restart-issue

…eviewer." This reverts commit ca5bca5.

This reverts commit 5295364.

… working" This reverts commit 0cff83a.

BenHenning · 2022-06-14T22:29:54Z

Thanks @KevinGitonga. Left one comment--please also add tests once we land on a final solution.

Note that in the future I suggest only sending a PR for review once it's completed from your perspective (which includes adding tests). If you'd like first thoughts on a PR as you're finishing (e.g. before tests are written), I suggest marking the PR as a 'draft' and explicitly mentioning when assigning reviewers that you'd like their initial thoughts. This helps better communicate expectations and ensure that reviewers are giving you the type of feedback that you're looking for.

… clicks.

oppiabot · 2022-06-29T02:27:59Z

Hi @KevinGitonga, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

…ely to make it clear.

… ExplorationActivityTest.kt.

BenHenning

Thanks @KevinGitonga. Just had a few follow-up comments--PTAL.

app/src/main/res/layout-sw600dp/story_fragment.xml

app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt

WORKSPACE

app/build.gradle

app/src/main/java/org/oppia/android/app/topic/TopicFragmentPresenter.kt

KevinGitonga · 2022-09-27T12:14:36Z

@BenHenning have added tests and shallow_since config PTAL.

BenHenning

Thanks @KevinGitonga! I think this has turned into quite a nice solution. LGTM.

BenHenning · 2022-09-29T00:59:17Z

@rt4914 since you requested changes, could you please take another look?

Dismissing since Rajat is out, and original concerns seem addressed.

BenHenning · 2022-09-29T01:43:50Z

Actually, dismissing @rt4914's review since his original concerns seem addressed and he's on holiday at the moment.

Rajat: if you have any follow-up concerns, please be sure to let @KevinGitonga know.

Going ahead and merging this.

This reverts commit 846657e. Conflicts: app/src/main/res/layout/license_text_viewer_activity.xml app/src/main/res/layout/revision_card_activity.xml

) ## Explanation Fixes #4709 This reverts commit 846657e from PR #4392. See #4709 (comment) for a detailed explanation for why this reversion is the correct approach. Note that this PR will be cherry-picked into the 0.10 release branch for the upcoming MR2 beta release. ## Essential Checklist - [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 This is a mostly clean reversion of #4392, so see that PR for an idea on how marquees behaved prior to that PR (as this PR returns the app to that behavior).

KevinGitonga added 5 commits June 7, 2022 11:00

Fix oppia#4186 Uncheck all selection on developer options not working

0cff83a

Fix oppia#4186 Add changes to code as suggested on code review.

5295364

Fix oppia#4186 Undo changes on line wrap as advised by code reviewer.

ca5bca5

Fix oppia#2581 Marquee restarts automatically in certain scenarios.

20e93e4

Fix oppia#2581 Marquee restarts automatically in certain scenarios.

841a35e

KevinGitonga requested a review from rt4914 as a code owner June 13, 2022 07:30

KevinGitonga assigned BenHenning Jun 13, 2022

rt4914 previously requested changes Jun 13, 2022

View reviewed changes

rt4914 assigned KevinGitonga and unassigned BenHenning Jun 13, 2022

KevinGitonga added 6 commits June 14, 2022 09:37

Fix oppia#2581 Marquee restarts automatically in certain scenarios.

f82b874

Fix oppia#2581 Marquee restarts automatically in certain scenarios.

008333c

Merge remote-tracking branch 'origin/fix-marquee-auto-restart-issue' …

d4f6dd5

…into fix-marquee-auto-restart-issue

Revert "Fix oppia#4186 Undo changes on line wrap as advised by code r…

3e99863

…eviewer." This reverts commit ca5bca5.

Revert "Fix oppia#4186 Add changes to code as suggested on code review."

4a60eac

This reverts commit 5295364.

Revert "Fix oppia#4186 Uncheck all selection on developer options not…

94222cf

… working" This reverts commit 0cff83a.

KevinGitonga assigned rt4914 and BenHenning and unassigned KevinGitonga Jun 14, 2022

BenHenning changed the title ~~Fix marquee auto restart issue~~ Fix #2581: Marquee auto restart issue Jun 14, 2022

BenHenning assigned KevinGitonga and unassigned BenHenning Jun 14, 2022

Insert timer in OnClick listener to enable unselect during subsequent…

c8b74e8

… clicks.

KevinGitonga assigned BenHenning Jun 17, 2022

BenHenning removed their assignment Jun 22, 2022

oppiabot bot added stale Corresponds to items that haven't seen a recent update and may be automatically closed. and removed stale Corresponds to items that haven't seen a recent update and may be automatically closed. labels Jun 29, 2022

KevinGitonga assigned BenHenning Jun 29, 2022

KevinGitonga added 4 commits September 22, 2022 10:17

Merge branch 'oppia:develop' into fix-marquee-auto-restart-issue

79e2e5b

Update speed and pause to animationSpeed and animationDelay respectiv…

7e4a505

…ely to make it clear.

Change Bazel workspace config to point to Oppia's official fork.

c89aadb

Change to click on MarqueeView as it is parent to the ToolbarTitle on…

24782a3

… ExplorationActivityTest.kt.

KevinGitonga assigned BenHenning and unassigned KevinGitonga Sep 24, 2022

BenHenning reviewed Sep 26, 2022

View reviewed changes

BenHenning assigned KevinGitonga and unassigned BenHenning Sep 26, 2022

KevinGitonga added 5 commits September 27, 2022 10:35

Add indentations to layouts and shallow_since from CI logs.

d8fb232

Revert shallow since.

0aa662d

Update shallow since.

d3b5b6c

Add class assertion tests.

58f10eb

Fix spacing NIT.

5f8afdd

KevinGitonga assigned BenHenning and unassigned KevinGitonga Sep 27, 2022

BenHenning approved these changes Sep 29, 2022

View reviewed changes

BenHenning assigned rt4914 and unassigned BenHenning Sep 29, 2022

BenHenning merged commit 846657e into oppia:develop Sep 29, 2022

vrajdesai78 mentioned this pull request Oct 6, 2022

[A11YS] Improve Text scaling of Marquee text in toolbar title #4640

Closed

BenHenning mentioned this pull request Nov 15, 2022

Exploration titles aren't showing up #4709

Closed

BenHenning added a commit that referenced this pull request Nov 17, 2022

Revert "Fix #2581: Marquee auto restart issue (#4392)"

66d7ba7

This reverts commit 846657e. Conflicts: app/src/main/res/layout/license_text_viewer_activity.xml app/src/main/res/layout/revision_card_activity.xml

BenHenning mentioned this pull request Nov 17, 2022

Fix #4709: Revert "Fix #2581: Marquee auto restart issue (#4392)" #4730

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #2581: Marquee auto restart issue #4392

Fix #2581: Marquee auto restart issue #4392

KevinGitonga commented Jun 13, 2022 •

edited

Loading

rt4914 left a comment

BenHenning commented Jun 14, 2022

oppiabot bot commented Jun 29, 2022

BenHenning left a comment

KevinGitonga commented Sep 27, 2022

BenHenning left a comment

BenHenning commented Sep 29, 2022

BenHenning commented Sep 29, 2022

Fix #2581: Marquee auto restart issue #4392

Fix #2581: Marquee auto restart issue #4392

Conversation

KevinGitonga commented Jun 13, 2022 • edited Loading

Explanation

Essential Checklist

For UI-specific PRs only

Before changes demonstration

After changes demonstration

After changes RTL demonstration

rt4914 left a comment

Choose a reason for hiding this comment

BenHenning commented Jun 14, 2022

oppiabot bot commented Jun 29, 2022

BenHenning left a comment

Choose a reason for hiding this comment

KevinGitonga commented Sep 27, 2022

BenHenning left a comment

Choose a reason for hiding this comment

BenHenning commented Sep 29, 2022

BenHenning commented Sep 29, 2022

KevinGitonga commented Jun 13, 2022 •

edited

Loading