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

Preparation for grids mako removal #4101

Merged
merged 7 commits into from Jun 2, 2017

Conversation

Projects
None yet
2 participants
@guerler
Copy link
Contributor

commented May 22, 2017

This PR enhances the grids helper such that grids can be built without using makos. Instead of returning an HTML string controller endpoints may now optionally return JSON. This allows us to transform grids one-by-one to the new mako-less format. This PR demonstrates this strategy with the published page list grid. A major difference between the previous HTML string / mako model and this structure here is that all grid operations will become async in the process hence avoiding page reloads. Another benefit is that the grid is loaded without the iframe. Once all grids operate without makos, the next step would be to transfer the pagination to the client side and continuously degrade the content provided through the response dictionary until the client side can take over and build grids through regular API endpoints.

@guerler guerler added this to the 17.09 milestone May 22, 2017

@guerler guerler changed the title Remove grids mako Preparation for grids mako removal May 22, 2017

guerler added some commits May 22, 2017

@guerler guerler added status/review and removed status/WIP labels May 23, 2017

@guerler guerler requested a review from dannon May 31, 2017

@dannon
Copy link
Member

left a comment

Looks good, just a few minor comments, only one of which really requires any addressing (the log statement) here.

@@ -304,7 +304,7 @@ return {
'<input type="hidden" id="operation" name="operation" value="">' +
'<td></td>' +
'<td colspan="100">' +
'For <span class="grid-selected-count"></span> selected ' + options.get_class_plural + ': ';
'For <span class="grid-selected-count"></span> selected items: ';

This comment has been minimized.

Copy link
@dannon

dannon Jun 2, 2017

Member

Talked to @guerler briefly about this and it's just going to be a temporary minor regression; we'll bring the model pluralization back in a subsequent change.

@@ -18,8 +19,36 @@ return Backbone.View.extend({
// Initialize
initialize: function(grid_config)
{
this.dict_format = grid_config.dict_format;
var self = this;
window.add_tag_to_grid_filter = function( tag_name, tag_value ){

This comment has been minimized.

Copy link
@dannon

dannon Jun 2, 2017

Member

We'll want this scoped more locally in the future.

'allowed' : operation.allowed(item),
'url_args' : url( **operation.get_url_args( item ) )
}
grid_config['items'].append(item_dict)
trans.log_action( trans.get_user(), text_type( "grid.view" ), context, params )

This comment has been minimized.

Copy link
@dannon

dannon Jun 2, 2017

Member

This log statement only happens for new-style (dict_format) now -- it should probably happen in both branches. (edit: Oops, it is, I just missed it working in my test -- disregard!)

@dannon dannon merged commit b764d35 into galaxyproject:dev Jun 2, 2017

5 checks passed

api test Build finished. 276 tests run, 0 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 150 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 34 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 579 tests run, 0 skipped, 0 failed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.