Skip to content
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

Allow user update slice title in visualize flow #3466

Merged

Conversation

graceguo-supercat
Copy link

@graceguo-supercat graceguo-supercat commented Sep 14, 2017

  1. after user make sql query and visualize, allow user click title to update slice title, and create a new slice at the same time.

  2. don't save new title if it is empty. When user click-out and title is not on focus, we will still show old title.

@graceguo-supercat graceguo-supercat changed the title Allow user create update slice title in visualize flow Allow user update slice title in visualize flow Sep 14, 2017
@coveralls
Copy link

coveralls commented Sep 14, 2017

Coverage Status

Coverage increased (+0.2%) to 69.164% when pulling da680e52f19fa94e15588444d03d6245d40d488f on graceguo-supercat:gg-AllowUpdateTitleOnVisualize into ad604ae on apache:master.

@graceguo-supercat graceguo-supercat force-pushed the gg-AllowUpdateTitleOnVisualize branch from da680e5 to 3e9b3fd Compare September 15, 2017 00:44
@coveralls
Copy link

coveralls commented Sep 15, 2017

Coverage Status

Coverage remained the same at 69.168% when pulling 3e9b3fd797f08c5163462305b2c959349c48eec7 on graceguo-supercat:gg-AllowUpdateTitleOnVisualize into e399a8c on apache:master.

this.props.actions.updateChartTitle(newTitle);
.then((data) => {
if (isNewSlice) {
window.location = data;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is a round trip necessary? As a user it may be confusing.

Copy link
Author

@graceguo-supercat graceguo-supercat Sep 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after new slice is created, we render "favorite" and "edit slice properties" icon on top of the slice. For chart in visualize flow we don't show those icons.
Refresh page after save slice_name is just to render those icons.

After discussing with Max, i will change response for saveSlice to return json formatted data (with new slice id), and rerender newly-created slice without refresh.

… update slice title, and create a new slice at the same time.

2. don't save new title if it is empty. Will still show old title.
@graceguo-supercat graceguo-supercat force-pushed the gg-AllowUpdateTitleOnVisualize branch 2 times, most recently from 26fae09 to 5085203 Compare September 21, 2017 06:12
@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.535% when pulling 50852037e9ee5c8e9562fae26ff7f1269d11ca90 on graceguo-supercat:gg-AllowUpdateTitleOnVisualize into 9af34ba on apache:master.

3 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.535% when pulling 50852037e9ee5c8e9562fae26ff7f1269d11ca90 on graceguo-supercat:gg-AllowUpdateTitleOnVisualize into 9af34ba on apache:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.535% when pulling 50852037e9ee5c8e9562fae26ff7f1269d11ca90 on graceguo-supercat:gg-AllowUpdateTitleOnVisualize into 9af34ba on apache:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.535% when pulling 50852037e9ee5c8e9562fae26ff7f1269d11ca90 on graceguo-supercat:gg-AllowUpdateTitleOnVisualize into 9af34ba on apache:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 69.386% when pulling 50852037e9ee5c8e9562fae26ff7f1269d11ca90 on graceguo-supercat:gg-AllowUpdateTitleOnVisualize into 9af34ba on apache:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 69.386% when pulling 50852037e9ee5c8e9562fae26ff7f1269d11ca90 on graceguo-supercat:gg-AllowUpdateTitleOnVisualize into 9af34ba on apache:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 69.386% when pulling 50852037e9ee5c8e9562fae26ff7f1269d11ca90 on graceguo-supercat:gg-AllowUpdateTitleOnVisualize into 9af34ba on apache:master.

@graceguo-supercat graceguo-supercat force-pushed the gg-AllowUpdateTitleOnVisualize branch from 5085203 to c696942 Compare September 21, 2017 18:21
@coveralls
Copy link

coveralls commented Sep 21, 2017

Coverage Status

Coverage remained the same at 69.535% when pulling c696942 on graceguo-supercat:gg-AllowUpdateTitleOnVisualize into 9af34ba on apache:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.535% when pulling c696942 on graceguo-supercat:gg-AllowUpdateTitleOnVisualize into 9af34ba on apache:master.

@mistercrunch mistercrunch merged commit 3949d39 into apache:master Sep 25, 2017
@graceguo-supercat graceguo-supercat deleted the gg-AllowUpdateTitleOnVisualize branch September 26, 2017 18:18
timifasubaa pushed a commit to timifasubaa/incubator-superset that referenced this pull request Oct 3, 2017
* 1. after user make sql query and visualize, allow user click title to update slice title, and create a new slice at the same time.
2. don't save new title if it is empty. Will still show old title.

* change saveSlice call response and update explore view
michellethomas pushed a commit to michellethomas/panoramix that referenced this pull request May 24, 2018
* 1. after user make sql query and visualize, allow user click title to update slice title, and create a new slice at the same time.
2. don't save new title if it is empty. Will still show old title.

* change saveSlice call response and update explore view
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.20.1 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.20.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants