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

Fixes 10334,BZ1217694 - Working indicator in products repository list #5268

Merged

Conversation

adamruzicka
Copy link
Member

gif here

@ehelms
Copy link
Member

ehelms commented Jun 4, 2015

A thought, if we converted this repository list to the details-nutupane like we have in other places (and thus enable search and what not) we should get loading and other functionality all in one. Thoughts?

@adamruzicka
Copy link
Member Author

I somehow missed details-nutupane. It would probably be better to use it. I'll rewrite it and report back

@ehelms
Copy link
Member

ehelms commented Jun 9, 2015

[test]

@ehelms
Copy link
Member

ehelms commented Jun 9, 2015

@adamruzicka it looks like there are various instances of the HTML not being aligned properly

@adamruzicka
Copy link
Member Author

@ehelms You mean the buttons? I tried investigating what would be needed to align them correctly. Apparently the buttons and the "x Selected" text are wrapped in a div with class "col-sm-4" which just doesnt give them enough space to be aligned properly.

With that class removed it looks like this but it might break other pages using details-nutupane

@ehelms
Copy link
Member

ehelms commented Jun 15, 2015

@adamruzicka sorry, I meant the raw HTML when viewing the file itself -- in some cases things are indented 2 spaces, some 4 and some not at all.

@adamruzicka
Copy link
Member Author

oh, sorry for misunderstanding, I'll fix that (along with the failing tests) tomorrow morning

@@ -49,8 +49,8 @@ angular.module('Bastion.products').controller('ProductRepositoriesController',
$scope.errorMessages = [];

$scope.checksums = [{name: translate('Default'), id: null}, {id: 'sha256', name: 'sha256'}, {id: 'sha1', name: 'sha1'}];
$scope.repositoriesTable = repositoriesNutupane.table;
$scope.repositoriesTable.removeRow = repositoriesNutupane.removeRow;
$scope.detailsTable = repositoriesNutupane.table;
Copy link
Member

Choose a reason for hiding this comment

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

need to add:

repositoriesNutupane.masterOnly = true;

otherwise it throws javascript errors

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't met any errors even without this line. Could you please check it again with the latest commit if you still see any errors?

Copy link
Member

Choose a reason for hiding this comment

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

you get this error when performing a search

@jlsherrill
Copy link
Member

Missed a reference to repositoriesTable (that was renamed to detailsTable in order to use the template):

https://github.com/Katello/katello/blob/master/engines/bastion_katello/app/assets/javascripts/bastion_katello/repositories/new/new-repository.controller.js#L19

I got a traceback after creating a repo due to it

@adamruzicka adamruzicka force-pushed the 10334-product_repository_loading branch from 014fb23 to 183f54b Compare June 22, 2015 07:20
@adamruzicka
Copy link
Member Author

@jlsherrill fixed the references I missed previously

@jlsherrill
Copy link
Member

@adamruzicka all looks good to me, mind squashing your commits?

@adamruzicka adamruzicka force-pushed the 10334-product_repository_loading branch from 183f54b to 89ac737 Compare June 23, 2015 07:02
@adamruzicka
Copy link
Member Author

@jlsherrill Squashed, thanks

@adamruzicka
Copy link
Member Author

@jlsherrill fixed the search traceback

@ehelms
Copy link
Member

ehelms commented Jul 7, 2015

[test]

@adamruzicka adamruzicka force-pushed the 10334-product_repository_loading branch from aa507d3 to cbaa0a8 Compare July 8, 2015 10:12
@ehelms
Copy link
Member

ehelms commented Jul 9, 2015

@jlsherrill awaiting a final ACK/merge from you as the original reviewer

@jlsherrill
Copy link
Member

ACK thanks @adamruzicka !

jlsherrill added a commit that referenced this pull request Jul 13, 2015
…ading

Fixes 10334,BZ1217694 - Working indicator in products repository list
@jlsherrill jlsherrill merged commit 1b051d8 into Katello:master Jul 13, 2015
@adamruzicka adamruzicka deleted the 10334-product_repository_loading branch July 13, 2015 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants