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

Setting titles on old and new katello pages #3666

Merged
merged 2 commits into from Feb 7, 2014

Conversation

waldenraines
Copy link
Contributor

Todo

  • Add tests

@@ -1,3 +1,5 @@
<span page-title="{{ 'Subscriptions for: ' | translate }} {{ system.name }}" model="system"></span>
Copy link
Member

Choose a reason for hiding this comment

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

This is not as clean as I might have hoped. Does the form below not work for this purpose?

<span page-title translate>Subscriptions for {{ system.name }}</span>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I didn't think about using transclusion, I could try that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have this working:

<span page-title ng-model="system">System {{ system.name }}</span>

But in order to get the translate directive to work we'd have to add another element or use the wonky syntax above because of this:

Error: [$compile:multidir] Multiple directives [translate, pageTitle] asking for transclusion on: <span page-title="" translate="" ng-model="system">

Either of these work:

<span page-title ng-model="system">{{ 'System' | translate }} {{ system.name }}</span>
<span translate><span page-title ng-model="system">System {{ system.name }}</span></span>

I couldn't get translate inside of page-title to work, however.

So do you have a preference?

@waldenraines
Copy link
Contributor Author

@ehelms updated.

@@ -63,7 +66,7 @@ angular.module('Bastion.test-mocks').factory('MockResource', ['$q', function($q)
$delete: function(callback) {
callback();
},
$promise: $q.defer().promise
$promise: deferred.promise
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure we want the same promise for every mockResource?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about that, but it should be fine since a new MockResource is injected each time in the beforeEach()s.

Copy link
Member

Choose a reason for hiding this comment

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

I was more thinking of the case where you had multiple objects in your collection that are all generated from this template resource.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed in IRC, will update to use mock $q.

@ehelms
Copy link
Member

ehelms commented Feb 7, 2014

ACK

waldenraines pushed a commit that referenced this pull request Feb 7, 2014
Setting titles on old and new katello pages
@waldenraines waldenraines merged commit c8b0f27 into Katello:master Feb 7, 2014
@waldenraines waldenraines deleted the set-title branch February 7, 2014 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants