Edit Dashboard title and Slice title in place #2940
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add EditableTitle component into Dashboard and Explore view to support edit title inline.
added 2 commits
June 9, 2017 17:04
Add EditableTitle component into Dashboard and Explore view to support edit title inline.
Add EditableTitle component into Dashboard and Explore view to support edit title inline.
added 2 commits
June 12, 2017 16:23
Add EditableTitle component into Dashboard and Explore view to support edit title inline.
Add EditableTitle component into Dashboard and Explore view to support edit title inline.
mistercrunch
requested changes
Jun 13, 2017
| ); | ||
| } | ||
| } | ||
| EditableTitle.propTypes = { |
There was a problem hiding this comment.
No big deal but we've been setting propTypes and defaultProps at the top of the file in most components.
| @@ -503,7 +533,8 @@ def test_dashboard_with_created_by_can_be_accessed_by_public_users(self): | |||
| ) | |||
| self.grant_public_access_to_table(table) | |||
|
|
|||
| dash = db.session.query(models.Dashboard).filter_by(dashboard_title="Births").first() | |||
| dash = db.session.query(models.Dashboard).filter_by( | |||
There was a problem hiding this comment.
nit: I've been breaking long sqlalchemy queries method-chaining-style, but in python it needs parenthesis as in:
dash = (
db.session.query(models.Dashboard)
.filter_by(slug="births")
.first()
)
| @@ -145,6 +150,19 @@ class ChartContainer extends React.PureComponent { | |||
| this.props.actions.runQuery(this.props.formData, true); | |||
| } | |||
|
|
|||
| updateChartTitle(newTitle) { | |||
| const params = {}; | |||
There was a problem hiding this comment.
nit: this is a bit more elegant:
const params = {
slice_name: newTitle,
action: 'overwrite',
form_data = this.props.formData,
};
| params.action = 'overwrite'; | ||
| params.form_data = this.props.formData; | ||
| const saveUrl = '/superset/explore/' + | ||
| `${this.props.datasourceType}/${this.props.datasourceId}/?${serialize(params)}`; |
There was a problem hiding this comment.
Maybe we can reuse the getExploreUrl method here? https://github.com/airbnb/superset/blob/master/superset/assets/javascripts/explore/exploreUtils.js#L4
Or maybe it's time soon to remove move save/overwrite out of /superset/explore/ and into its own POST endpoint.
superset/assets/utils/common.js
Outdated
| @@ -86,3 +86,33 @@ export function getShortUrl(longUrl, callback) { | |||
| }, | |||
| }); | |||
| } | |||
|
|
|||
| export function serialize(obj) { | |||
There was a problem hiding this comment.
I'm hoping we can get away without this function by reusing getExploreUrl
Most notably Flask AppBuilder to 1.9.0
Moved the histogram to be rooted on the right side as well fixes apache#2933
* Fix handling of Chunked requests Add fix for handling chunk encoding requests. If ENABLE_CHUNK_ENCODING is set to true, for requests with transfer encoding set to true. It will set wsgi.input_terminated to true which tells werkzeug to ignore content-length and read the stream till the end. break comment in multiple lines * remove debug print logging
* [dashboard] notify instead of modal onSave * Addressing comments
* get compiled js file names * make manifest available as template var * use script src directly to avoid flash of unstyled content in the case of csstheme.js * linting * attempt to fix tests * exception * print the path when no manifest file found * handle case when manifest.json is not present for some reason, or in the case of tests
* sort explicitly on label * add simple test for /slicemodelview/add endpoint * make comments and method names more clear * fix test name * be more explicit, test status_code
mistercrunch
approved these changes
Jun 14, 2017
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add EditableTitle component into Dashboard and Explore view to support edit title inline.
fixes #2796
dashboard view:
Explore view:
