-
Notifications
You must be signed in to change notification settings - Fork 5
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
Adds core datasets location filter #116
Conversation
app/scripts/actions/links.js
Outdated
@@ -43,6 +44,8 @@ export function updateURL() { | |||
}); | |||
} | |||
|
|||
if (locationFilter) query += `&coreDatasetsLocation=${locationFilter}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we should manage the URL with a library (That's a thing that you are always telling me)... https://www.npmjs.com/package/query-string
@@ -13,7 +14,7 @@ import DatasetsList from '../ui/DatasetsList'; | |||
import { TABS_OPTIONS } from '../../constants'; | |||
|
|||
|
|||
class DataMap extends React.Component { | |||
class ExploreSidebar extends React.Component { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you....(I can't believe that DataMap was its previous name)
.c-dataset-location-filter { | ||
padding: 0 40px; | ||
|
||
.c-tabs { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that this should be a theme instead of changing it only at this point
export const setDatasetsLocation = createThunkAction('datasets/onSetDatasetsLocation', location => | ||
(dispatch) => { | ||
dispatch(setLocation(location)); | ||
dispatch(updateURL()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that this would be re-usable as long as you are going to always update the URL... Which one? always the explore one? What if you need to update another URL? What if you don't need to update an URL?
{g.subgroups.map((sg) => { | ||
const list = data.filter((d) => { | ||
const locationFilter = isLocationGlobal ? | ||
true : (((d.vocabulary[0] || {}).attributes || {}).tags || []).includes(location); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bulletproof but illegible
Task: https://www.pivotaltracker.com/story/show/153742502