search

WebMemex / webmemex-extension

📇 Your digital memory extension, as a browser extension
https://webmemex.org
Other
208 stars 45 forks source link

Represent search query in the url of overview #64 #69

Closed Rohanhacker closed 7 years ago

Rohanhacker commented 7 years ago

2

Treora commented 7 years ago

I finally got around to look at this PR. Nice work, it seems to work as intended. I did however wonder about the maintainability of the code, since care has to be taken that both the window location and the Redux state keep the same value. The logic for this has been put into the overview component, while conceptually it has very little reason to be there: it is a problem of syncing redux state and window location. Your approach matches a more commonly suggested approach (e.g. here), but starting from this idea I decided to try to decouple this logic from the UI, and make an abstraction that will also allow us to easily expose other variables from the state to the location. See PR #92 for the result. Closing this one. Thanks nevertheless!