Fixes #4231: Adding support for svgz images #4862

supreme96 · 2023-01-29T09:43:51Z

Explanation

Fixes #4231: Adding support for svgz images

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

SVGZ file used : https://raw.githubusercontent.com/Rioiv/svgz/16d5417b6bfcb18ea9e1d354e7dda3e73abad18f/1.svgz

(manual test) current behavior of app loading a png file in the lesson.

(manual test) loading an svgz file without the change.

(manual test) successfully loading the svgz file with the change.

the new unit tests failing without the change.

supreme96 · 2023-01-31T08:54:52Z

@BenHenning @seanlip PTAL.

BenHenning

Thanks @supreme96! I think this looks quite good! Just had a few comments--PtAL.

I'm also updating this to the latest develop and kicking off CI so that we can get CI results.

@seanlip are there any SVGZ images on the Oppia web side that we might be able to manually point to in order to test & verify that they can be loaded correctly in the pipeline (now that they're being supported)? If not, @supreme96 I suggest perhaps temporarily updating calls to loadDrawable() to force-load any sample SVGZ file from the internet, and then showing via a video that it loads correctly in the app (please include a link to the SVGZ file itself for reference). It'd be especially helpful to see a before & after video (that is, a video showing that the image does not load correctly before these fixes are in place).

Also, @supreme96 could you update the PR description to include screenshots of the new tests failing without the UrlImageParser changes in place? It's good to verify that the tests fail when expected so that we know they'll catch regressions for this logic in the future.

utility/src/main/java/org/oppia/android/util/parser/image/UrlImageParser.kt

utility/src/test/java/org/oppia/android/util/parser/image/UrlImageParserTest.kt

seanlip · 2023-02-01T17:04:20Z

@seanlip are there any SVGZ images on the Oppia web side that we might be able to manually point to in order to test & verify that they can be loaded correctly in the pipeline (now that they're being supported)?

No, not yet, sorry. @supreme96, I would suggest following Ben's suggestion here. Thanks!

supreme96 · 2023-02-02T16:37:36Z

Thanks @supreme96! I think this looks quite good! Just had a few comments--PtAL.

I'm also updating this to the latest develop and kicking off CI so that we can get CI results.

@seanlip are there any SVGZ images on the Oppia web side that we might be able to manually point to in order to test & verify that they can be loaded correctly in the pipeline (now that they're being supported)? If not, @supreme96 I suggest perhaps temporarily updating calls to loadDrawable() to force-load any sample SVGZ file from the internet, and then showing via a video that it loads correctly in the app (please include a link to the SVGZ file itself for reference). It'd be especially helpful to see a before & after video (that is, a video showing that the image does not load correctly before these fixes are in place).

Also, @supreme96 could you update the PR description to include screenshots of the new tests failing without the UrlImageParser changes in place? It's good to verify that the tests fail when expected so that we know they'll catch regressions for this logic in the future.

@BenHenning Can you help me figure out where we are displaying svgs in the app? I can then test the change manually. I will also upload the requested screenshots and videos.

supreme96 · 2023-02-02T16:38:05Z

@BenHenning Need some help. PTAL at the comment above.

seanlip · 2023-02-02T18:24:25Z

Any images in the "lesson" parts of the app are typically SVGs. E.g. the thumbnail images for graphics, the images in lessons, etc.

You'll need to look for these in the data supplied with the code, though. The above only explains how you can see an SVG image visually when you run Oppia Android locally.

BenHenning · 2023-02-07T03:27:48Z

Adding to @seanlip's comment, but you can look for oppia-noninteractive-image tags within the existing fractions or ratios lessons that are included in the app locally (see src/domain/main/assets) and either change one of those, or update one of the other lessons to include a tag pointing to svgz to show that it can load correctly.

I highly suggest verifying the code both with and without the fix in place (without the image shouldn't show up at all--this is a good verification to take a screenshot of to demonstrate that the lesson was, indeed, updated to use svgz).

supreme96 · 2023-02-12T18:36:59Z

Adding to @seanlip's comment, but you can look for oppia-noninteractive-image tags within the existing fractions or ratios lessons that are included in the app locally (see src/domain/main/assets) and either change one of those, or update one of the other lessons to include a tag pointing to svgz to show that it can load correctly.

I highly suggest verifying the code both with and without the fix in place (without the image shouldn't show up at all--this is a good verification to take a screenshot of to demonstrate that the lesson was, indeed, updated to use svgz).

Sure. I'll try that. Thanks :D

oppiabot · 2023-02-19T18:54:32Z

Hi @supreme96, 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!

## Explanation Fixes oppia#4765 Demo Video: https://user-images.githubusercontent.com/64526117/210055420-016f2dd2-46d6-4166-ae2a-be4c98710299.mp4 This PR tries to handle the problem of the continue interaction button's animation not being eye-catching to the users. Currently, it does so by changing the animation to a shake animation. The button now does a little wobble, starting 45s after the screen is shown. It wobbles 2 times, and then repeats the animation after every 8 seconds. Only with further testing and trial, will we know how the animation appeals to the users and resolves the problem at hand. **Technical detail:** The anim file (xml) is set to play the animation 2 times. In the `ContinueButtonView.kt` file, the startAnimating() function has been made recursive, which calls itself after every 8 seconds in order to repeat the animation. The 8 second gap is achieved using the lifeCycleSafeTimerFactory. ## 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

## Explanation Fix oppia#1801 This fix adds check in the implemented playStatusLiveData to complete audioPause request if any is pending while the playerStatus keeps changing. When user clicks on pause audio icon and the UI disappears there is a delay in audio actually pausing. Adding a check in playStatusLiveData completes pause pending request fast enough to prevent it from playing. ## 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: ...".) - [ ] 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  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 changes demo https://user-images.githubusercontent.com/20886444/192735111-6baf46f8-3531-49f8-83d6-98277dc74c6d.mp4 ## After changes demo https://user-images.githubusercontent.com/20886444/192735968-13b7e2b5-e296-4978-9194-4df5b807897b.mp4 --------- Co-authored-by: Ben Henning <[email protected]>

## Explanation Dark mode implementation - Fixed Post PR Changes Part 2 This PR changes following Dark Mode Post PR :- 1. Check dark-mode light mode staples for all radio-buttons: ReadingTextSize,AudioLanguage,AppLanguage,etc. 2. Merge `component_color_shared_multipane_icon_color`and `component_color_lessons_tab_activity_lesson_card_drop_down_arrow_color` 3. Try removing `component_color_profile_chooser_activity_white_text_color`  ## 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 ### Radio Buttons <img src="https://user-images.githubusercontent.com/76530270/216781619-8be9b4f2-6126-4613-9649-4dbc06a9bb76.png" height="400" style="max-width: 100%"> <img src="https://user-images.githubusercontent.com/76530270/216781614-837af21b-45b0-49fe-8f86-de63d5620b68.png" height="400" style="max-width: 100%"> <img src="https://user-images.githubusercontent.com/76530270/216781689-6ad86984-f2ff-42e3-866a-6d287adf8f2f.png" height="400" style="max-width: 100%"> <img src="https://user-images.githubusercontent.com/76530270/216781692-55068024-bbf2-46ed-baf5-c5abe47feb89.png" height="400" style="max-width: 100%"> <img src="https://user-images.githubusercontent.com/76530270/216781743-f0042e74-dfeb-4ab3-bf94-d3c90b0735c3.png" height="400" style="max-width: 100%"> <img src="https://user-images.githubusercontent.com/76530270/216781747-c20f2205-5d60-4551-8ba1-da708308c55b.png" height="400" style="max-width: 100%">  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

…agments) (oppia#4870)  ## Explanation Dark mode implementation - Non Visible Layouts (Topic and Practice Fragments)  ## 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: ...".) - [ ] 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 ### Info Fragment <img src="https://user-images.githubusercontent.com/76530270/216989746-47b0dc3c-7073-49e1-898a-c698c176b13c.png" height="400" style="max-width: 100%"> <img src="https://user-images.githubusercontent.com/76530270/216989760-b4df59ba-3d3f-4a49-8592-fa916cf1d70c.png" height="400" style="max-width: 100%"> ### Practice Fragment <img src="https://user-images.githubusercontent.com/76530270/216990071-20b23217-4f6d-459a-b73b-81e3d9f7f203.png" height="400" style="max-width: 100%"> <img src="https://user-images.githubusercontent.com/76530270/216990075-0b9b7b45-5879-46a9-8cf1-a7e6c62bb19a.png" height="400" style="max-width: 100%"> <img src="https://user-images.githubusercontent.com/76530270/216990380-f7e51039-f6e7-4c8b-b8c1-3198f1bba5dc.png" height="400" style="max-width: 100%"> <img src="https://user-images.githubusercontent.com/76530270/216990382-1a5512db-7c3d-448b-bb9d-e64480dea914.png" height="400" style="max-width: 100%">  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

@rt4914

Remove code owner @rt4914 from all non-xml files

…ntation. (oppia#4876)  ## Explanation Fixed oppia#4147 : Remove colors_migrating.xml post dark mode implementation.  ## 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  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

supreme96 · 2023-02-24T13:03:51Z

Hi @BenHenning PTAL, adding screenshots as requested.

SVGZ file used : https://raw.githubusercontent.com/Rioiv/svgz/16d5417b6bfcb18ea9e1d354e7dda3e73abad18f/1.svgz

(manual test) current behavior of app loading a png file in the lesson.

(manual test) loading an svgz file without the change.

(manual test) successfully loading the svgz file with the change.

the new unit tests failing without the change.

supreme96 · 2023-02-24T13:05:27Z

Thanks @supreme96! I think this looks quite good! Just had a few comments--PtAL.

I'm also updating this to the latest develop and kicking off CI so that we can get CI results.

@seanlip are there any SVGZ images on the Oppia web side that we might be able to manually point to in order to test & verify that they can be loaded correctly in the pipeline (now that they're being supported)? If not, @supreme96 I suggest perhaps temporarily updating calls to loadDrawable() to force-load any sample SVGZ file from the internet, and then showing via a video that it loads correctly in the app (please include a link to the SVGZ file itself for reference). It'd be especially helpful to see a before & after video (that is, a video showing that the image does not load correctly before these fixes are in place).

Also, @supreme96 could you update the PR description to include screenshots of the new tests failing without the UrlImageParser changes in place? It's good to verify that the tests fail when expected so that we know they'll catch regressions for this logic in the future.

Added.

oppiabot · 2023-02-24T13:06:55Z

Unassigning @supreme96 since a re-review was requested. @supreme96, please make sure you have addressed all review comments. Thanks!

BenHenning

Thanks @supreme96! I think this looks really good--well done on the screenshots demonstrating the behavior. My only nit is that the screenshots should go in the PR description (but I went ahead and did this to avoid needing another back-and-forth on the PR).

Just waiting on CI to pass, but otherwise the PR LGTM.

oppiabot · 2023-02-25T03:45:12Z

Unassigning @BenHenning since they have already approved the PR.

supreme96 added 2 commits January 29, 2023 00:17

Adding check for supporting svgz images in UrlImageParser.

fdd9b6b

Merge remote-tracking branch 'origin' into svg-support

db70784

supreme96 requested a review from BenHenning as a code owner January 29, 2023 09:43

oppiabot bot assigned BenHenning and seanlip Jan 31, 2023

BenHenning requested changes Jan 31, 2023

View reviewed changes

utility/src/main/java/org/oppia/android/util/parser/image/UrlImageParser.kt Outdated Show resolved Hide resolved

utility/src/test/java/org/oppia/android/util/parser/image/UrlImageParserTest.kt Outdated Show resolved Hide resolved

Merge branch 'develop' into svg-support

8c3ceb3

BenHenning assigned supreme96 and unassigned BenHenning Jan 31, 2023

seanlip removed their assignment Feb 1, 2023

supreme96 removed their assignment Feb 2, 2023

oppiabot bot assigned BenHenning and seanlip Feb 2, 2023

seanlip removed their assignment Feb 4, 2023

BenHenning assigned supreme96 and unassigned BenHenning Feb 7, 2023

oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Feb 19, 2023

supreme96 and others added 5 commits February 24, 2023 18:16

Incorporating PR suggestions

30c22ff

rt4914 and others added 2 commits February 24, 2023 18:16

Remove code owner @rt4914 from all non-xml files (oppia#4875)

9c65e3b

Remove code owner @rt4914 from all non-xml files

supreme96 requested review from rt4914 and a team as code owners February 24, 2023 12:48

oppiabot bot removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Feb 24, 2023

Merge branch 'oppia:develop' into svg-support

8f27270

supreme96 requested review from BenHenning and removed request for rt4914 February 24, 2023 13:04

oppiabot bot assigned BenHenning and unassigned supreme96 Feb 24, 2023

oppiabot bot assigned seanlip Feb 24, 2023

seanlip removed their assignment Feb 24, 2023

Merge branch 'develop' into svg-support

4fba898

BenHenning approved these changes Feb 25, 2023

View reviewed changes

BenHenning enabled auto-merge (squash) February 25, 2023 03:43

oppiabot bot unassigned BenHenning Feb 25, 2023

oppiabot bot added the PR: LGTM label Feb 25, 2023

BenHenning merged commit 2eed01e into oppia:develop Feb 25, 2023

supreme96 deleted the svg-support branch February 25, 2023 10:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #4231: Adding support for svgz images #4862

Fixes #4231: Adding support for svgz images #4862

supreme96 commented Jan 29, 2023 •

edited by BenHenning

Loading

supreme96 commented Jan 31, 2023

BenHenning left a comment

seanlip commented Feb 1, 2023

supreme96 commented Feb 2, 2023

supreme96 commented Feb 2, 2023

seanlip commented Feb 2, 2023

BenHenning commented Feb 7, 2023

supreme96 commented Feb 12, 2023

oppiabot bot commented Feb 19, 2023

supreme96 commented Feb 24, 2023

supreme96 commented Feb 24, 2023

oppiabot bot commented Feb 24, 2023

BenHenning left a comment

oppiabot bot commented Feb 25, 2023

Fixes #4231: Adding support for svgz images #4862

Fixes #4231: Adding support for svgz images #4862

Conversation

supreme96 commented Jan 29, 2023 • edited by BenHenning Loading

Explanation

Essential Checklist

For UI-specific PRs only

(manual test) current behavior of app loading a png file in the lesson.

(manual test) loading an svgz file without the change.

(manual test) successfully loading the svgz file with the change.

the new unit tests failing without the change.

supreme96 commented Jan 31, 2023

BenHenning left a comment

Choose a reason for hiding this comment

seanlip commented Feb 1, 2023

supreme96 commented Feb 2, 2023

supreme96 commented Feb 2, 2023

seanlip commented Feb 2, 2023

BenHenning commented Feb 7, 2023

supreme96 commented Feb 12, 2023

oppiabot bot commented Feb 19, 2023

supreme96 commented Feb 24, 2023

(manual test) current behavior of app loading a png file in the lesson.

(manual test) loading an svgz file without the change.

(manual test) successfully loading the svgz file with the change.

the new unit tests failing without the change.

supreme96 commented Feb 24, 2023

oppiabot bot commented Feb 24, 2023

BenHenning left a comment

Choose a reason for hiding this comment

oppiabot bot commented Feb 25, 2023

supreme96 commented Jan 29, 2023 •

edited by BenHenning

Loading