Skip to content

Commit

Permalink
Fix #4622: Android Wiki migration (#4893)
Browse files Browse the repository at this point in the history
<!-- 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 #4622
- The PR does the following: This PR moves the android wiki into the
oppia-andoird codebase allowing contributors to make PRs to the wiki
along with changes to the codebase itself. This PR also introduces a
GitHub Action (Runs when a PR is merged into develop and when a user
makes change to the wiki via the web interface) that copies the content
of the wiki folder along with the git history of the wiki folder and
deploys it to the oppia-android wiki repository.

The steps followed to achieve this wiki migration are present here: 
[Oppia Android Wiki Migration V2
(1).pdf](https://github.com/oppia/oppia-android/files/10921370/Oppia.Android.Wiki.Migration.V2.1.pdf)

### Note: Please ensure GitHub actions have write permissions. Please
follow these instructions to enable it:

[GrantingReadandWritePermissionsonGitHub_PDF.pdf](https://github.com/oppia/oppia-android/files/10919834/GrantingReadandWritePermissionsonGitHub_PDF.pdf)


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

---------

Co-authored-by: Rajat Talesra <[email protected]>
Co-authored-by: Ben Henning <[email protected]>
Co-authored-by: Akshay Nandwana <[email protected]>
Co-authored-by: Srikanth K B <[email protected]>
Co-authored-by: Jan <[email protected]>
Co-authored-by: Abhay Garg <[email protected]>
Co-authored-by: Arjun Gupta <[email protected]>
Co-authored-by: Sparsh Agrawal <[email protected]>
Co-authored-by: Farees Hussain <[email protected]>
Co-authored-by: Sarthak Agarwal <[email protected]>
Co-authored-by: Sean Lip <[email protected]>
Co-authored-by: Apoorv Srivastava <[email protected]>
Co-authored-by: Vinita Murthi <[email protected]>
Co-authored-by: Ankita Saxena <[email protected]>
Co-authored-by: Vojtěch Jelínek <[email protected]>
Co-authored-by: Ben Henning <[email protected]>
  • Loading branch information
17 people authored Mar 17, 2023
1 parent e042b4f commit 57ec240
Show file tree
Hide file tree
Showing 50 changed files with 3,980 additions and 1 deletion.
3 changes: 3 additions & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ gradlew.bat @BenHenning
/.github/workflows/ @BenHenning
/.github/stale.yml @BenHenning

# Wiki
/wiki/ @BenHenning

# Devbots configurations.
/.devbots/ @BenHenning

Expand Down
2 changes: 1 addition & 1 deletion .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@
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 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
27 changes: 27 additions & 0 deletions .github/workflows/wiki.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
name: Deploy to Wiki
on:
push:
branches:
- develop
paths:
- 'wiki/**'
# Triggers this workflow when the wiki is changed
# (see https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#gollum).
gollum:

jobs:
wiki-deploy:
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-20.04]
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 0
- name: Add remote
run: |
git filter-branch --subdirectory-filter wiki/ -- --all
git remote set-url origin https://github.com/oppia/oppia-android.wiki.git
git checkout -b master
git push origin master --force
21 changes: 21 additions & 0 deletions scripts/assets/todo_open_exemptions.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -290,3 +290,24 @@ todo_open_exemption {
exempted_file_path: "scripts/src/java/org/oppia/android/scripts/todo/model/Todo.kt"
line_number: 10
}

todo_open_exemption {
exempted_file_path: "wiki/Coding-style-guide.md"
line_number: 33
}

todo_open_exemption {
exempted_file_path: "wiki/Static-Analysis-Checks.md"
line_number: 155
line_number: 163
line_number: 168
line_number: 172
line_number: 176
line_number: 178
line_number: 193
line_number: 203
line_number: 206
line_number: 209
line_number: 212
line_number: 215
}
144 changes: 144 additions & 0 deletions wiki/Accessibility-A11y-Guide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
## Overview
Accessibility is an important part of Oppia to ensure that the app is accessible by everyone. Some common conditions that affect a person's use of an Android device are:
* blindness / low vision
* deafness / impaired hearing
* cognitive disabilities
* confined motor skills
* color blindness, etc.

Making sure that the Oppia app is accessible by all resonates with overall Oppia's mission: **to help anyone learn anything they want in an effective and enjoyable way.**

Note: In short we can write Accessibility as **A11Y**.

## How to test the app for a11y users?
There are various manual and automated tests to check if app is accessible by all or not. All of the below mentioned tests are required to make sure that app is accessible in most cases.

**[Accessibility Scanner](https://support.google.com/accessibility/android/answer/6376570?hl=en)** : Using Accessibility Scanner we can take screenshots of each and every screen in the Oppia-app manually and the Accessibility Scanner app will give the output for the individual screenshot mentioning all the errors.

**Screen Reader**: Screen readers like **Talkback** can be used to test the app manually. Talkback app is used by blind people to navigate to different items in the screen and get audio based output. This app will not give any error like Accessibility Scanner.

**[AccessibilityChecks](https://developer.android.com/guide/topics/ui/accessibility/testing#automated)**: Developers can activate the `AccessibilityChecks` in all `Espresso` test cases which will give errors related to accessibility.


## Setting up Accessibility Scanner and Talkback

### Setup Play Store in mobile emulator
1. Create a new emulator device which contains **Google Play Store** in it. Example: Nexus 5, Nexus 5A, Pixel, Pixel 2, Pixel 3, Pixel 3a, Pixel 4.
2. Open "Play Store" app and sign-in.

### Setup Play Store in tablet emulator
By default tablet emulators do not contain **Play Store** app and therefore you will need to make some changes to make it available.
1. Follow the steps mentioned [here](https://stackoverflow.com/a/62680014).
2. Once the above steps are done, start the emulator.
3. Open the "Play Store" app and sign-in.

### Using a11y scanner in android
[Accessibility Scanner](https://support.google.com/accessibility/android/answer/6376570?hl=en) scans your screen and provides suggestions to improve the accessibility of your app, based on:
* Content labels
* Touch target size
* Clickable items
* Text and image contrast

#### How to Use?
1. Open **Google Play Store**
2. Download/Install **Accessibility Scanner** app
3. After installation, open Settings
4. Search **Accessibility Scanner**, click on it.
5. Turn on **Use Accessibility Scanner** -> **Allow**
6. You will notice a blue colored floating button with tick/check icon.
7. Open **Oppia** app.
8. Now on any screen inside app click on the floating button and either record or take snapshot.

**Result**: You will notice that the scanner analyses the screen and give errors if there are any or else it will show that no errors were found.

### Using Talkback in android
TalkBack is the Google **screen reader** included on Android devices. TalkBack gives you spoken feedback so that you can use your device without looking at the screen.

#### How to use?
1. Open **Google Play Store**
2. Download/Install **Android Accessibility Suite** app
3. After installation, open Settings
4. Search **Talkback**, click on it.
5. Read all the instructions written on the screen as using Talkback requires specific steps.
6. Turn on **Use Service** -> **Allow**


### Useful Resources
* [Android A11Y Overview](https://support.google.com/accessibility/android/answer/6006564)
* [Using A11Y Menu](https://support.google.com/accessibility/android/answer/9078941)
* [Getting started with Talkback](https://support.google.com/accessibility/android/answer/6283677)
* [Display speech output as Text: Talkback](https://developer.android.com/guide/topics/ui/accessibility/testing#optional_talkback_developer_settings)


## Using AccessibilityTestRule in Espresso Tests
[AccessibilityTestRule](https://github.com/oppia/oppia-android/blob/develop/testing/src/main/java/org/oppia/android/testing/AccessibilityTestRule.kt) is a JUnit rule to enable `AccessibilityChecks` in all Espresso Tests. This rule covers all errors shown by Accessibility Scanner and more but only for all those UI elements which are getting used in the test case.

(**Note: If this file is not available then it has been merged with OppiaTestRule as per #3351**)

#### How to use?
Simply use the `AccessibilityTestRule` in Espresso Test file like this:
```
@get: Rule
val accessiblityTestRule = AccessibilityTestRule()
```

This will enable the `AccessibilityChecks` on all Espresso tests inside this file.

To disable `AccessibilityChecks` on any individual test case use [DisableAccessibilityChecks](https://github.com/oppia/oppia-android/blob/develop/testing/src/main/java/org/oppia/android/testing/DisableAccessibilityChecks.kt) as annotation with the test.

In case of test failure there are two options to fix it:
* Solve the test case by updating the UI as per the error details.
* If by solving the error the user experience will become worse than we should actually suppress that error instead of changing the UI. This can be done in [AccessibilityTestRule](https://github.com/oppia/oppia-android/blob/fe553d32e0161f6efa6e465109306b909dbcc476/testing/src/main/java/org/oppia/android/testing/AccessibilityTestRule.kt#L34)

## Auditing the app
The app should be audited by covering different use cases across 23 different manual tests mentioned [here](https://docs.google.com/spreadsheets/d/1lFQo2XE0dSGZcMvr7paxdL3zXB3FVcRnZOqD70DT3a4/edit?usp=sharing).
This sheet has been divided based on `primary` and `secondary` use cases and `basic` and `advanced` test cases.
* Level 1 = `Primary + Basic`, which means that all primary use cases passes all `Basic` tests.
* Level 2 = `Secondary + Basic`, which means that all secondary use cases passes all `Basic` tests.
* Level 3 = `Primary + Advanced`, which means that all primary use cases passes all `Advanced` tests.
* Level 4 = `Secondary + Advanced`, which means that all secondary use cases passes all `Advanced` tests.

This entire sheet should be filled with each release as a part of audit process.

## General Tips to make app Accessible
* All Clickable items should have a minimum size of `48x48dp`.
* Buttons should use `android:enabled` instead of `android:clickable` to disable/enable it.
* All views should have a `foreground:background` contrast ratio of `4.5:1` and above.
* Texts should have a minimum `12sp` text size.
* Images/icons should have a meaningful content description.

## Exceptional Cases
* Generally we use `sp` only for text-size/font-size of text and at all attributes related to width/height we use `dp`. But we can use width in `sp` if we have text inside a fixed width container. This will increase the size of container whenever we increase the font size, so the scaled text get enough size to fit inside a container. If this case is applied anywhere in UI, please get confirmation from @BenHenning or @rt4914 .

## Android 12 Warnings around TextViews in Fixed Layouts

### Problem with fixed layout

If we have scalable text inside a fixed width container then accessibility scanner is suggesting to improve text scaling, as if the text scales it won’t get enough space to expand inside a fixed width container.

<img width="250" src="https://user-images.githubusercontent.com/9396084/200759127-f8f9b4e5-1017-4e24-b0c2-28179a520aa7.png"/>

### Possible solution to fix it

1. Change the fixed width to wrap_content and set minWidth. In this case, we can’t directly change the width into a wrap_content as all thumbnail images won’t have consistent width which leads to a problem as mentioned in issue [#4684](https://github.com/oppia/oppia-android/issues/4684). You can see the below screenshot for reference.

<img width="250" src="https://user-images.githubusercontent.com/9396084/200760708-ba6d2659-9cc1-4701-a93d-8647d7db1335.png" />

2. As directly we can’t use wrap_content for width, another possible solution is to use scalable width i.e. instead of setting fixed width in dp we can set the width in sp. This will increase the container size every time we increase the font size, so the scaled text can fit inside the container. In this approach, accessibility scanner will still show suggestion to improve text scaling but from a UI perspective this approach works. For reference you can look at PR [#4695](https://github.com/oppia/oppia-android/pull/4695)

3. Another approach would be to design such that we don’t have to fit a text inside a fixed width container as shown below in the reference image.

<img width="250" src="https://user-images.githubusercontent.com/9396084/200761059-27e0e9be-0fed-4f01-beac-9dff61ab7563.png" />

We can set the full width to the cards i.e. match_parent (with appropriate margins) which will remove the issue of accessibility. We can also show the dots at bottom which represent the number + position of items.

<img width="250" src="https://user-images.githubusercontent.com/9396084/200761183-0562e48a-259f-4488-8020-11bfb5065f08.png" />

For sighted users
- The banners will be cyclic i.e. item-0, item1, item2, item0 and repeat.

For talkback users
- The cyclic nature will stop.
- The screen reader will start from item-0, item-1, item-2, and next it will go out of list.


46 changes: 46 additions & 0 deletions wiki/Background-Processing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
## Overview

This page aims to provide some context around:
- What constitutes background processing/when something should be background processed, and why we care
- How background processing is done in the Android app
- Best practices to follow when managing background or expensive tasks
- Existing utilities to simplify using DataProviders
- How to safely pass data to the UI

## Background

Performing asynchronous tasks or background processing in a multi-threaded environment is hard: most developers are unaware of the nuances of cross-thread development (e.g. properly utilizing critical sections without introducing deadlocks, sharing mutable state across thread boundaries with correctly applied memory fences, critical sections, or atomics, properly using concurrent data structures, and more). This problem is exacerbated in Android since:
1. Android requires all user-facing operations to be run on the main thread (requiring communication to/from the main thread)
2. Android UI objects are very [lifecycle](https://developer.android.com/guide/components/activities/activity-lifecycle)-sensitive (meaning haphazard management of Android state can at best leak memory or at worst crash when communicating back from a background thread--a common crash in Android apps)
3. The Android UI thread is sensitive to even medium-length operations when on slow devices (which can lead to app [ANR](https://developer.android.com/topic/performance/vitals/anr)s)

The team has a number of carefully considered solutions to ensure concurrency is easier to manage, safer, and performant.

### Definition of background processing & when to use it

All features in the codebase can be represented as a data pipeline. In some cases, data is created transiently and in other cases it needs to be loaded from somewhere (e.g. a file or network). Further, we sometimes need to do a lot of processing on this data before it can be presented to the UI (the app's [architecture](https://github.com/oppia/oppia-android/wiki/Overview-of-the-Oppia-Android-codebase-and-architecture#app-architecture) is specifically designed to encourage data processing logic to live outside the UI).

To keep things simple, we consider everything the following to be worth executing on a background thread instead of the UI thread:
- Any logic operation (e.g. something requiring an if statement or loop) which is more complicated than just copying data
- Any file I/O operations (e.g. reading from a file)
- Any networking operations (e.g. calling into Retrofit)
- Complex state management (such as [ExplorationProgressController](https://github.com/oppia/oppia-android/blob/a85399c2b0a2b9cf214881ce8c70d9b487f1e0b8/domain/src/main/java/org/oppia/android/domain/exploration/ExplorationProgressController.kt#L34))

Similarly, the following operations must happen on a UI thread for lifecycle safety reasons:
- Any interactions with the UI (e.g. activities, fragments, or views)
- Any interactions with ViewModel state (which is designed to only be mutated on the main thread)
- Any interactions with other Android services which require main thread access

### Why we care

Oppia Android is aiming to provide an effective education experience to the most underprivileged communities in the world, and this particularly requires excellent performance on low-end devices. We have a thin performance margin to operate in, and we can't afford ANRs or poor performance. Further, reducing crashes is important to ensure an uninterrupted learning experience (especially for children who might not understand how to recover the app from a crash).

That being said, the difficulty in writing correct & performant concurrent code is quite high. We want to make sure we achieve that with a lower barrier-to-entry so that team members don't have to manage especially complex code.

## Background processing & concurrency in Oppia Android

To ensure the team is meeting the goals of reducing concurrency complexity while not sacrificing performance or correctness, we require that all code utilize the patterns & best practices outlined in this section.

- [Kotlin Coroutines](https://github.com/oppia/oppia-android/wiki/Kotlin-Coroutines)
- [DataProvider & LiveData](https://github.com/oppia/oppia-android/wiki/DataProvider-&-LiveData)
- [PersistentCacheStore & In Memory Blocking Cache](https://github.com/oppia/oppia-android/wiki/PersistentCacheStore-&-In-Memory-Blocking-Cache)
Loading

0 comments on commit 57ec240

Please sign in to comment.