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.

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

fix(openchallenges): improve query parameter management using location service #2273

Merged
merged 17 commits into from
Oct 27, 2023

Conversation

rrchai
Copy link
Contributor

@rrchai rrchai commented Oct 25, 2023

Changelogs

  • Replace router.navigate with location.replaceState() to update url params without initiating navigation or scroll restoration
  • Trigger API call by updating query once params are updated
  • Refactor search pages' *_component.ts files
    • challenge-search: 608 -> 397 lines
    • org-search: 332 -> 316 lines (could reduce more by moving codes of generating orgCard to a global service, but save for a future pr)

@rrchai rrchai self-assigned this Oct 25, 2023
Copy link
Member

@tschaffter tschaffter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code refactoring looks great!

I'm having a CORS issue on my end that prevent the challenges from loading in the search page. I will check if this issue is also in main.

UPDATE: The CORS issue was because a few ports were not properly forwarded by VS Code, including the port of the API gateway and challenge service. Stopping the forwarding of these ports and forwarding them again using VS Code UI solved the issue (see #2279 for more details).

@tschaffter
Copy link
Member

@rrchai Filtering orgs by "Contribution Role" does not filter the results.

@tschaffter tschaffter self-requested a review October 26, 2023 17:45
Copy link
Member

@tschaffter tschaffter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment above.

@rrchai rrchai requested a review from tschaffter October 27, 2023 11:19
@tschaffter tschaffter merged commit a3716c1 into Sage-Bionetworks:main Oct 27, 2023
6 checks passed
@rrchai rrchai deleted the fix-scroll branch January 10, 2024 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Reset scrolling position when navigating to another page (Take 2)
2 participants