Prevent page refresh if present result is same as result to be searched#1249
Conversation
|
Thanks for creating the patch. Can you instead change this code to use navigateToUrl and change the implementation of navigateToUrl to check if the URL has changed? |
|
Ohh ok @jcscottiii , you mean instead of using window.location.href, I should call the navigateToUrl function right? |
suprith-hub
force-pushed
the
prevent-refresh-search-button-click
branch
from
7016a65 to
4cfe1ca
Compare
|
@jcscottiii I have made the changes |
|
@jcscottiii I have completed the changes... |
jcscottiii
left a comment
jcscottiii
left a comment
There was a problem hiding this comment.
Apologies for the tardy review on this. Thanks for moving the check inside navigateToUrl. I have some suggestions to make. Then this should be good to go.
Let's move the usage of window.location.origin inside of navigateToUrl.
In the future, I want our formatXxxUrl helpers to return AppLocation instead of string. AppLocation will have things like the search and pathname
Then we can change navigateToUrl to accept AppLocation instead of string. And in navigateToUrl, we can easily assemble the new location.href with something like
const fullNewUrl = window.location.origin + location.pathname + (location.search || '') + (location.hash || '');
frontend/src/static/js/components/webstatus-overview-filters.ts
Outdated
Show resolved
Hide resolved
|
Have you tried running out playwright test suite locally? It seems to be failing. I haven't had a chance to try it yet. |
@jcscottiii I have tried using the documentation also I tried running the workflow directly. |
|
Are you able to run this inside the dev container? That should provide the same environment used for development and in CI. |
@jcscottiii I was able to run playwright tests in devcontainer. I could not understand how the 3 tests failed in my devcontainer but passed in Github for 2nd time... |
|
No worries! I think it was something intermittent going on that was unrelated to your change. I am queueing the pull request now. Thanks for the contribution! |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
2 participants
Hi @jcscottiii, this is a enhancement, addressing the issue: #1237
I am also working to immediately reflect the disabled state(low opacity and hover disable) of
search Icon, will be able to make a PR on i soon, also can I do draft PR and ask for assistance?