-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
[explore-v2] add fave star and edit button to chart header #1623
Conversation
This PR needs to be linted to pass the build |
} | ||
|
||
onClick(e) { | ||
e.preventDefault(); |
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 am curious, what is default behavior?
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.
since we are using an <a href="#">
here, we want to stop the default behaviour of the browser following the href path, in this case just adding #
to the url.
https://developer.mozilla.org/en/docs/Web/API/Event/preventDefault
} | ||
|
||
export default function TooltipWrapper({ label, tooltip, children, placement}) { | ||
console.log('children', children) |
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.
leftovers ?
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.
yep. looks like my linter stopped working in sublime for some reason, so i missed this. thanks for the 👀
if (data.count > 0) { | ||
dispatch(toggleFaveStar(true)); | ||
} | ||
}); |
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.
codeclimate says: Missing semicolon.
@@ -49,6 +53,33 @@ export function fetchFieldOptions(datasourceId, datasourceType) { | |||
}; | |||
} | |||
|
|||
export const FETCH_FAVE_STAR = 'FETCH_FAVE_STAR' |
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.
codeclimate says: Missing semicolon.
const url = `${FAVESTAR_BASE_URL}/${sliceId}/count`; | ||
$.get(url, (data) => { | ||
if (data.count > 0) { | ||
dispatch(toggleFaveStar(true)); |
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.
codeclimate says: "toggleFaveStar" was used before it was defined
} | ||
} | ||
|
||
export const SAVE_FAVE_STAR = 'SAVE_FAVE_STAR' |
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.
codeclimate says: Missing semicolon.
const urlSuffix = isStarred ? 'unselect' : 'select'; | ||
const url = `${FAVESTAR_BASE_URL}/${sliceId}/${urlSuffix}/`; | ||
$.get(url); | ||
dispatch(toggleFaveStar(!isStarred)); |
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.
codeclimate says: "toggleFaveStar" was used before it was defined
const url = `${FAVESTAR_BASE_URL}/${sliceId}/${urlSuffix}/`; | ||
$.get(url); | ||
dispatch(toggleFaveStar(!isStarred)); | ||
} |
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.
codeclimate says: Missing semicolon.
@@ -1,6 +1,10 @@ | |||
/* eslint camelcase: 0 */ | |||
const $ = window.$ = require('jquery'); | |||
|
|||
// todo: fix bug in favstar endpoint that requires | |||
// that /undefined/ is part of this url. | |||
const FAVESTAR_BASE_URL = '/superset/favstar/undefined'; |
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.
s/undefined/Slice:
@expose("/favstar/<class_name>/<obj_id>/<action>/")
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.
thanks @bkyryliuk!
export const FETCH_FAVE_STAR = 'FETCH_FAVE_STAR' | ||
export function fetchFaveStar(sliceId) { | ||
return function (dispatch) { | ||
const url = `${FAVESTAR_BASE_URL}/${sliceId}/count`; |
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.
there is no such action is count, I believe you can keep it with no action and the endpoint will return you the count
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.
👍
6ec36df
to
258ef2b
Compare
plz review @vera-liu @mistercrunch @bkyryliuk