-
Notifications
You must be signed in to change notification settings - Fork 349
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
Added better configuration of buttons in 'control' field. #317
base: master
Are you sure you want to change the base?
Conversation
}, | ||
|
||
editTemplate: function() { | ||
return this._createUpdateButton().add(this._createCancelEditButton()); | ||
return this.editButton ? this._createUpdateButton().add(this._createCancelEditButton()) : $(""); |
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.
Not sure, why do we need to hide update & cancel button for edit row?
Initial idea was hiding edit button, but still being able to edit a row (e.g. with row click). And to complete editing or cancel changed we need these buttons.
The changes don't make sense if you only have a single Control field - it makes more sense if you have more than one, like my example of moving the delete button to it's own column. When doing that, default behaviour results in a somewhat quirky looking UI. Have a look at the Plunker from the first post, and compare the UI behaviour of the first and last versions - especially around toggling insert and edit modes. I guess I could have made a custom delete-button-only field type, but it seemed like an OK idea to add more customisation to the existing control. |
Agree, this makes sense. |
I think the search (or filter, really) and clear-filter buttons logically belong together, so it would probably make sense to drop clearFilterButton and tie those code paths to searchButton instead. But that would break backwards compatibility for people using that option - I've strived to not make breaking changes :) |
The last point before we can merge it: |
Added toggles for searchButton and editButton for the control field type, and change the editTemplate so it only has the update/cancel buttons if editButton is true.
This might seem like an odd idea at first, but is useful if you want to have control buttons outside the grid, or want several control fields with different functionality. I have a feature request for moving the delete button to a separate column, because users hit it by accident after cancelling an edit.
A demonstration is available here - the oldest version uses version 1.4.1 of js-grid, which has slightly weird UX, while the most recent Plunk version uses js-grid built from my feature branch.