NIAEFEUP / nijobs-fe

[FRONTEND] A platform for companies to advertise their job opportunities to students
GNU General Public License v3.0
23 stars 3 forks source link

Change source of URL params #317

Open Naapperas opened 1 year ago

Naapperas commented 1 year ago

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.