You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
#251 introduced SearchURLWidget which gives the user the ability to preview and copy the their clipboard their current search link. However, since we are using react-router, perhaps we should use the exported useLocation hook to get the location information instead of relying on browser APIs.
One pro of this solution is that we do not mix APIs (since we are already using react-router) and some weird errors can be avoided since they should be caught upstream by the dependency: #316 had some issues regarding this since window.location became non-assignable (I suspect some changes in some of our test dependencies caused this).
However, one con of this is that tests that somehow exercise the use of the hook need to have access to react-router's internal context through the use of Routers: this pattern is already used in some places, so anyone who looks at this already has some examples of this. One other way of tackling this is to mock the useLocation hook: I did not manage to make this work but I welcome anyone to try. If you do so I also invite whoever takes up on this work to replace the "enclose with Router" test components with the "mock hook" approach.
This issue talked about a lot of possible changes but is mainly related to investigation work and to see what is best for our use case.
The text was updated successfully, but these errors were encountered:
#251 introduced
SearchURLWidget
which gives the user the ability to preview and copy the their clipboard their current search link. However, since we are usingreact-router
, perhaps we should use the exporteduseLocation
hook to get the location information instead of relying on browser APIs.One pro of this solution is that we do not mix APIs (since we are already using
react-router
) and some weird errors can be avoided since they should be caught upstream by the dependency: #316 had some issues regarding this sincewindow.location
became non-assignable (I suspect some changes in some of our test dependencies caused this).However, one con of this is that tests that somehow exercise the use of the hook need to have access to
react-router
's internal context through the use ofRouter
s: this pattern is already used in some places, so anyone who looks at this already has some examples of this. One other way of tackling this is to mock theuseLocation
hook: I did not manage to make this work but I welcome anyone to try. If you do so I also invite whoever takes up on this work to replace the "enclose with Router" test components with the "mock hook" approach.This issue talked about a lot of possible changes but is mainly related to investigation work and to see what is best for our use case.
The text was updated successfully, but these errors were encountered: