Fix #15735 Remove the Refresh modal class #16378

Priyansh61 · 2022-10-21T20:23:32Z

Overview

This PR fixes Switching languages is not working appropriately while playing exploration(CUJ 1.16) on web and mobile #15735
This PR does the following: Fix the language change in exploration

Essential Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
The linter/Karma presubmit checks have passed locally on your machine.
"Allow edits from maintainers" is checked. (See here for instructions on how to enable it.)
- This lets reviewers restart your CircleCI tests for you.
The PR is made from a branch that's not called "develop".

Proof that changes are correct

language-2022-10-21_22.53.16.mp4

Proof of changes on desktop with slow/throttled network

Proof of changes on mobile phone

Proof of changes in Arabic language

PR Pointers

Make sure to follow the instructions for making a code change.
If you need a review or an answer to a question, and don't have permissions to assign people, leave a comment like the following: "{{Question/comment}} @{{reviewer_username}} PTAL". Oppiabot will help assign that person for you.
For what code owners will expect, see the Code Owner's wiki page.
Make sure your PR follows conventions in the style guide, otherwise this will lead to review delays.
Never force push. If you do, your PR will be closed.
Some of the e2e tests are flaky, and can fail for reasons unrelated to your PR. We are working on fixing this, but in the meantime, if you need to restart the tests, please check the "If your build fails" wiki page.

oppiabot · 2022-10-21T20:23:44Z

Hi, @Priyansh61, this pull request does not have a "CHANGELOG: ..." label as mentioned in the PR pointers. Assigning @Priyansh61 to add the required label. PRs without this label will not be merged. If you are unsure of which label to add, please ask the reviewers for guidance. Thanks!

Priyansh61 · 2022-10-22T13:59:57Z

@nithinrdy PTAL!

nithinrdy · 2022-10-25T05:18:44Z

Hey, could you please explain the ideal expected behaviour (the behaviour that this PR is trying to achieve) just for my understanding?

Priyansh61 · 2022-10-25T17:07:27Z

Thank you @nithinrdy for reviewing the PR.

Since a refresh is not required to change the language that appears to the user after selecting the language translation option, the PR eliminates the refresh functionality.

This does help to prevent unnecessary language switches to English when the user chooses another language.

In order to change the language without requiring a refresh, I removed the refresh modal class along with the tests and added some functionality.

I hope this helps 😄

oppiabot · 2022-10-26T01:33:25Z

Hi @Priyansh61, there is a new change in develop which needs to be in your PR. Please update your branch with the latest changes in develop. For instructions, refer to this link. Thanks!

nithinrdy · 2022-10-26T10:49:00Z

@Priyansh61 the issue description says that the issue arises when the user makes some progress (completes a state, for example), and then changes the language.

Select an option using radio button which redirects to next screen (i.e. complete a card/state)
Now switch to spanish language which auto-reloads the page with popup

The video in the PR description doesn't display that user journey. The language is changed before even the first card is completed.

Could you please upload a new video where you first complete a state and then change the language, and show that the issue no longer occurs?

Priyansh61 · 2022-10-28T13:22:50Z

@nithinrdy I just had a doubt, we cant change the language of a card when we move to the next one since there is no language selection option available on the new card.

language-2022-10-28_14.39.12.mp4

So is this what you were asking me to show ?? 😅

oppiabot · 2022-10-28T16:30:33Z

Hi @Priyansh61, the build of this PR is stale and this could result in tests failing in develop. Please update this pull request with the latest changes from develop. Thanks!

nithinrdy

Thanks! Left a comment.

...lates/pages/exploration-player-page/layout-directives/content-language-selector.component.ts

oppiabot · 2022-11-03T07:39:14Z

Hi @Priyansh61, the build of this PR is stale and this could result in tests failing in develop. Please update this pull request with the latest changes from develop. Thanks!

oppiabot · 2022-12-31T09:07:56Z

Hi @Priyansh61, the build of this PR is stale and this could result in tests failing in develop. Please update this pull request with the latest changes from develop. Thanks!

gp201

@Priyansh61 great job on the tests. Only some lint issues left. PTAL

gp201 · 2023-01-04T07:47:03Z

.../pages/exploration-player-page/layout-directives/content-language-selector.component.spec.ts

    ]);
  });

+  it('should correcly initialize newLanguageCode', () => {
+    windowRef = TestBed.inject(WindowRef);


Can you move this line to the beforeEach block?

.../pages/exploration-player-page/layout-directives/content-language-selector.component.spec.ts

gp201

I left a review above PTAL

…/content-language-selector.component.spec.ts Co-authored-by: Praneeth <praneethg2001@gmail.com>

Priyansh61 · 2023-01-05T10:02:19Z

@gp201 PTAL! I have addressed all the requested changes :)

gp201

LGTM

oppiabot · 2023-01-06T19:41:52Z

Hi @nithusha21, this PR is ready to be merged. Author of this PR does not have permissions to merge this PR. Before you merge it, please make sure that there are no pending comments that require action from the author's end. Thanks!

Priyansh61 requested a review from a team October 21, 2022 20:23

Priyansh61 requested a review from a team as a code owner October 21, 2022 20:23

Priyansh61 requested review from gp201 and nithinrdy October 21, 2022 20:23

oppiabot bot assigned Priyansh61 Oct 21, 2022

Priyansh61 added the PR CHANGELOG: Bug fixes -- @nithusha21 label Oct 21, 2022

Priyansh61 assigned nithinrdy and unassigned Priyansh61 Oct 22, 2022

nithinrdy assigned Priyansh61 and unassigned nithinrdy Oct 25, 2022

Priyansh61 assigned nithinrdy and unassigned Priyansh61 Oct 25, 2022

oppiabot bot assigned Priyansh61 Oct 26, 2022

nithinrdy removed their assignment Oct 26, 2022

oppiabot bot added the PR: don't merge - STALE BUILD label Oct 28, 2022

oppiabot bot removed the PR: don't merge - STALE BUILD label Oct 28, 2022

nithinrdy reviewed Oct 28, 2022

View reviewed changes

...lates/pages/exploration-player-page/layout-directives/content-language-selector.component.ts Outdated Show resolved Hide resolved

Priyansh61 requested a review from nithinrdy October 31, 2022 10:07

oppiabot bot added the PR: don't merge - STALE BUILD label Nov 3, 2022

oppiabot bot removed the PR: don't merge - STALE BUILD label Nov 3, 2022

Priyansh61 removed their assignment Nov 3, 2022

gp201 assigned Priyansh61 and unassigned gp201 Dec 31, 2022

Priyansh61 added 3 commits January 3, 2023 15:24

tests fixed

525cdda

lint check fixed

ebbc6c2

lint checks fixed

7cbeda7

oppiabot bot removed the PR: don't merge - STALE BUILD label Jan 3, 2023

Merge branch 'develop' into language

82ec367

Priyansh61 assigned gp201 and unassigned Priyansh61 Jan 3, 2023

gp201 approved these changes Jan 4, 2023

View reviewed changes

gp201 suggested changes Jan 4, 2023

View reviewed changes

gp201 assigned Priyansh61 and unassigned gp201 Jan 4, 2023

Priyansh61 and others added 6 commits January 5, 2023 13:04

Update core/templates/pages/exploration-player-page/layout-directives…

550ac16

…/content-language-selector.component.spec.ts Co-authored-by: Praneeth <praneethg2001@gmail.com>

moved the inject statement

a01dd1a

lint checks

b480635

Merge branch 'develop' into language

5e7af28

lint fix

6e2a27f

Merge branch 'language' of github.com:Priyansh61/oppia into language

47871e3

Priyansh61 requested a review from gp201 January 5, 2023 10:02

Priyansh61 assigned gp201 and unassigned Priyansh61 Jan 5, 2023

gp201 approved these changes Jan 6, 2023

View reviewed changes

gp201 assigned Shivkant-Chauhan and unassigned gp201 Jan 6, 2023

oppiabot bot added the PR: LGTM label Jan 6, 2023

seanlip merged commit 3bcd1fb into oppia:develop Jan 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #15735 Remove the Refresh modal class #16378

Fix #15735 Remove the Refresh modal class #16378

Priyansh61 commented Oct 21, 2022 •

edited by Lawful2002

Loading

oppiabot bot commented Oct 21, 2022

Priyansh61 commented Oct 22, 2022

nithinrdy commented Oct 25, 2022

Priyansh61 commented Oct 25, 2022 •

edited

Loading

oppiabot bot commented Oct 26, 2022

nithinrdy commented Oct 26, 2022

Priyansh61 commented Oct 28, 2022

oppiabot bot commented Oct 28, 2022

nithinrdy left a comment

oppiabot bot commented Nov 3, 2022

oppiabot bot commented Dec 31, 2022

gp201 left a comment

gp201 Jan 4, 2023

gp201 left a comment

Priyansh61 commented Jan 5, 2023 •

edited

Loading

gp201 left a comment

oppiabot bot commented Jan 6, 2023

Fix #15735 Remove the Refresh modal class #16378

Fix #15735 Remove the Refresh modal class #16378

Conversation

Priyansh61 commented Oct 21, 2022 • edited by Lawful2002 Loading

Overview

Essential Checklist

Proof that changes are correct

Proof of changes on desktop with slow/throttled network

Proof of changes on mobile phone

Proof of changes in Arabic language

PR Pointers

oppiabot bot commented Oct 21, 2022

Priyansh61 commented Oct 22, 2022

nithinrdy commented Oct 25, 2022

Priyansh61 commented Oct 25, 2022 • edited Loading

oppiabot bot commented Oct 26, 2022

nithinrdy commented Oct 26, 2022

Priyansh61 commented Oct 28, 2022

oppiabot bot commented Oct 28, 2022

nithinrdy left a comment

Choose a reason for hiding this comment

oppiabot bot commented Nov 3, 2022

oppiabot bot commented Dec 31, 2022

gp201 left a comment

Choose a reason for hiding this comment

gp201 Jan 4, 2023

Choose a reason for hiding this comment

gp201 left a comment

Choose a reason for hiding this comment

Priyansh61 commented Jan 5, 2023 • edited Loading

gp201 left a comment

Choose a reason for hiding this comment

oppiabot bot commented Jan 6, 2023

Priyansh61 commented Oct 21, 2022 •

edited by Lawful2002

Loading

Priyansh61 commented Oct 25, 2022 •

edited

Loading

Priyansh61 commented Jan 5, 2023 •

edited

Loading