Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Have the token respect flags on the service. #642

Merged
merged 1 commit into from Nov 6, 2014

Conversation

kadams54
Copy link
Contributor

@kadams54 kadams54 commented Nov 5, 2014

When a token renders, it should set the toggle state of its buttons based on how flags are set in the service it wraps.

@kadams54
Copy link
Contributor Author

kadams54 commented Nov 5, 2014

QA

With the as flag:

  1. Deploy the mongodb:cluster bundle.
  2. Switch to the added services sidebar.
  3. Set some services to hide, others to highlight. (Don't be concerned if nothing happens in the canvas, that's another card.)
  4. Close the added services sidebar.
  5. Reopen it and verify that all the buttons are still in the same hidden/highlighted state.

// would be to just use the service flags directly, but that's a
// far-reaching change that we want to tackle at a later point.
attrs.highlight = attrs.service.highlight;
attrs.visible = !attrs.service.fade;
Copy link
Contributor

Choose a reason for hiding this comment

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

would prefer an update to the template to look in the proper place instead of making new properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having the template use these properties won't work unless/until the token is updating the attributes directly. After a button click, the token is re-rendered, so the service attributes need to be set properly before that re-rendering happens. Unfortunately service attributes are updated asynchronously, in response to an event firing, which means they may not get set until after the re-render. And THAT means that we need to use local attributes for renders that follow a button click (i.e., don't override local attributes with service attributes).

@jujugui
Copy link
Contributor

jujugui commented Nov 5, 2014

Test FAILed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/2166/

@@ -99,12 +99,24 @@ YUI.add('juju-added-service-token', function(Y) {

@method render
*/
render: function() {
render: function(override) {
Copy link
Contributor

Choose a reason for hiding this comment

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

override attribute is missing a doc block.

I would prefer something like useServiceVisibility as override is very generic.

@jujugui
Copy link
Contributor

jujugui commented Nov 6, 2014

Test PASSed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/2177/

@hatched
Copy link
Contributor

hatched commented Nov 6, 2014

👍 QA OK Thanks for this fix

QA Notes:
Tested with ghost services as well as deployed services and bundles as per the qa outline.

@makyo
Copy link
Contributor

makyo commented Nov 6, 2014

👍 QA okay, thanks!

@kadams54
Copy link
Contributor Author

kadams54 commented Nov 6, 2014

Thanks all! :shipit:

@jujugui
Copy link
Contributor

jujugui commented Nov 6, 2014

Build failed: Attempt to land pull request failed
build url: http://ci.jujugui.org:8080/job/juju-gui-merge/764

@kadams54
Copy link
Contributor Author

kadams54 commented Nov 6, 2014

Spurious failure.

@jujugui
Copy link
Contributor

jujugui commented Nov 6, 2014

Status: merge request accepted. Url: http://ci.jujugui.org:8080/job/juju-gui-merge

@jujugui
Copy link
Contributor

jujugui commented Nov 6, 2014

Test PASSed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/2179/

jujugui added a commit that referenced this pull request Nov 6, 2014
Have the token respect flags on the service.

When a token renders, it should set the toggle state of its buttons based on how flags are set in the service it wraps.
@jujugui jujugui merged commit 4c85cac into juju:develop Nov 6, 2014
@jujugui
Copy link
Contributor

jujugui commented Nov 6, 2014

Test FAILed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/2180/

@kadams54 kadams54 deleted the as-service-visibility branch November 6, 2014 18:39
@jujugui
Copy link
Contributor

jujugui commented Nov 6, 2014

Test PASSed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/2181/

@jujugui
Copy link
Contributor

jujugui commented Nov 6, 2014

Test PASSed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/2183/

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

Successfully merging this pull request may close these issues.

None yet

4 participants