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

Replace MathExpressionParserViewModel html parser usage with better factory function #4206

Closed
BenHenning opened this issue Feb 18, 2022 · 0 comments · Fixed by #5277
Closed

Replace MathExpressionParserViewModel html parser usage with better factory function #4206

BenHenning opened this issue Feb 18, 2022 · 0 comments · Fixed by #5277
Assignees
@XichengSpencer
Labels
bug End user-perceivable behaviors which are not desirable. 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

@BenHenning
Copy link
Member

BenHenning commented Feb 18, 2022 •
edited by adhiamboperes
Loading

#2173 introduces a new MathExpressionParserViewModel that uses html parser factory in a context where images shouldn't be loaded. This flow was simplified in #3852, so this issue tracks updating the model to use the new factory method.

Create a new factory method in HtmlParser, just like the policies one. What is required here, is that the new create function is such that it does not request for the arguments that are not set currently here:

Screenshot 2023-12-08 at 01 30 21
@Broppia Broppia added issue_type_dev_initiated Impact: Low Low perceived user impact (e.g. edge cases). labels Jun 2, 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
@seanlip seanlip added bug End user-perceivable behaviors which are not desirable. and removed issue_type_infrastructure labels Mar 28, 2023
@adhiamboperes adhiamboperes added the Work: Low Solution is clear and broken into good-first-issue-sized chunks. label Aug 14, 2023
@XichengSpencer XichengSpencer self-assigned this Nov 21, 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 Dec 7, 2023
@XichengSpencer XichengSpencer mentioned this issue Dec 19, 2023
Merged
6 tasks
adhiamboperes added a commit that referenced this issue Jan 22, 2024
@XichengSpencer @adhiamboperes
Fix #4206 Create New HtmlParser Factory Method (#5277)
b173a2a 
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation

Fix #4206 Create New HtmlParser Factory Method to only take
customOppiaTagActionListener and displayLocale as parameter and replace
factory method usage in MathExpressionParserViewModel with this new
implementation
<!--
- 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.
  -->

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

- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing
![Screenshot 2024-01-07
142738](https://github.com/oppia/oppia-android/assets/74568012/70cb0448-2ed5-458f-bbba-cf8877e11ef1)

![Screenshot 2024-01-07
145604](https://github.com/oppia/oppia-android/assets/74568012/e465d9de-8153-456b-a1db-28428cc04ad8)

---------

Co-authored-by: Adhiambo Peres <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@XichengSpencer XichengSpencer

Labels
bug End user-perceivable behaviors which are not desirable. 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.
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 #4206 Create New HtmlParser Factory Method XichengSpencer/oppia-android
5 participants
@seanlip @BenHenning @adhiamboperes @XichengSpencer @Broppia