search

oppia / oppia

A free, online learning platform to make quality education accessible for all.
https://www.oppia.org
Apache License 2.0
5.63k stars 3.78k forks source link

Fix#19990 Make the dialogue modal disappear when user navigates to contributor profile page #20222

Closed cwasserman1 closed 2 weeks ago

cwasserman1 commented 2 weeks ago

Overview

  1. This PR fixes or fixes part of #19990.
  2. This PR does the following: In issue #19990, it was noted that when the user attempted to navigate to one of the contributor profile pages, the dialogue modal would not disappear as intended. This PR fixes that by calling the cancel() method, which closes the modal, from the HTML of the contributor navigation buttons.
  3. (For bug-fixing PRs only) The original bug occurred because: The dialogue modal needs to be explicitly closed, as it does not automatically close upon navigation. However, there was previously no such code to address this.

    Proof that changes are correct

Video of the bug before the change

https://github.com/oppia/oppia/assets/18372009/8234259a-35dd-47ae-9133-5775e1a11bb4

Proof of fix working after the change

https://github.com/oppia/oppia/assets/18372009/a834cc43-9b51-4f4d-acdd-1d9405581b3b

Proof of changes on desktop with slow/throttled network

No different behavior with 3G throttling

oppiabot[bot] commented 2 weeks ago

Hi @cwasserman1, can you complete the following:

  1. The body of this PR is missing the checklist section, please update it to include the checklist. Thanks!
oppiabot[bot] commented 2 weeks ago

Hi @cwasserman1 please assign the required reviewer(s) for this PR. Thanks!

cwasserman1 commented 2 weeks ago

@hrshkshri This PR is ready for review

hrshkshri commented 2 weeks ago

@hrshkshri This PR is ready for review

Hey @cwasserman1 Please make sure to assign reviewers for code owner reviews. (@Lawful2002 -- in this case)

cwasserman1 commented 2 weeks ago

@hrshkshri This PR is ready for review

Hey @cwasserman1 Please make sure to assign reviewers for code owner reviews. (@Lawful2002 -- in this case)

Ok, sorry about that. I will make sure to do that in the future. Does the PR look good?

oppiabot[bot] commented 2 weeks ago

Unassigning @Lawful2002 since they have already approved the PR.

oppiabot[bot] commented 2 weeks ago

Hi @cwasserman1, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to ask someone to merge your PR once the CI checks pass and you're happy with it. Thanks!

cwasserman1 commented 2 weeks ago

@hrshkshri @Lawful2002 Hi, would someone mind merging the PR for me please? I am unable to do it since I am not an authorized user. Thanks!

seanlip commented 2 weeks ago

@cwasserman1 I just put it in the merge queue. Congrats on your first PR to Oppia! :tada:

cwasserman1 commented 2 weeks ago

@cwasserman1 I just put it in the merge queue. Congrats on your first PR to Oppia! 🎉

thank you!