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

Textbox editing fix #1896

Merged
merged 3 commits into from Sep 13, 2017
Merged

Textbox editing fix #1896

merged 3 commits into from Sep 13, 2017

Conversation

alison985
Copy link
Contributor

Combines mozilla/redash PR’s 86 and 95.

There was a bug that saved textbox content on a dashboard when you
tried to close without saving. This fixes it.

Combines mozilla/redash PR’s 86 and 95.

There was a bug that saved textbox content on a dashboard when you
tried to close without saving. This fixes it.
Copy link
Member

@arikfr arikfr left a comment

Choose a reason for hiding this comment

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

Thanks! Please see comments.

@@ -8,7 +8,7 @@ const EditTextBoxComponent = {
close: '&',
dismiss: '&',
},
controller(toastr) {
controller($rootScope, $location, $http, toastr) {
Copy link
Member

Choose a reason for hiding this comment

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

Why the extra injects are needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They aren't. Good eye, thanks.

},
};

function DashboardWidgetCtrl($location, $uibModal, $window, Events, currentUser) {
this.canViewQuery = currentUser.hasPermission('view_query');

this.editTextBox = () => {
this.widget.existing_text = this.widget.text;
Copy link
Member

Choose a reason for hiding this comment

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

I think it will be better to keep this logic in the component. So the component will make a copy of the original value and restore to it when closed instead of saved.

Another option is to use a different object as the ng-model of the <textarea> and only on save assign it to the widget. This way no extra logic is needed on close and you can bring back keyboard support.

@alison985
Copy link
Contributor Author

@arikfr Updated.

@arikfr arikfr merged commit e76efc9 into getredash:master Sep 13, 2017
@arikfr
Copy link
Member

arikfr commented Sep 13, 2017

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants