Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(5116)

Issue 9650043: Disable building relations if charm is not loaded.

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 11 months ago by bac
Modified:
10 years, 11 months ago
Reviewers:
teknico, frankban, mp+165171
Visibility:
Public.

Description

Disable building relations if charm is not loaded. The potential endpoints for a service are not available until the charm is loaded. This branch fixes a bug where an attempt was made to access the non-existent endpoints and then blithely proceeded along. In addition, the UI is changed so that the 'Build Relation' submenu is disabled if the charm is not loaded (a proxy for the endpoints not being there) and the 'long-click-to-begin-relation-building' is also disabled. The whole relation building experience is up for redesign. At that time we may chose to do something that provides a better user experience, such as showing a spinner until the service is fully available. For now, the actual time the actions will be disabled should be a matter of a few seconds. A feature flag is introduced and can be used with the sandbox to artifically set the charm loading delay for testing. Use it as follows for a ten second delay before the charm is reported as loaded: http://localhost:8888/:flags:/charmLoadDelay=10000 https://code.launchpad.net/~bac/juju-gui/endpoints-delay/+merge/165171 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 9

Patch Set 2 : Disable building relations if charm is not loaded. #

Patch Set 3 : Disable building relations if charm is not loaded. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+170 lines, -29 lines) Patch
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M app/app.js View 1 chunk +1 line, -1 line 0 comments Download
M app/models/endpoints.js View 1 chunk +5 lines, -0 lines 0 comments Download
M app/store/env/fakebackend.js View 2 chunks +9 lines, -2 lines 0 comments Download
M app/store/env/simulator.js View 1 chunk +1 line, -1 line 0 comments Download
M app/views/topology/relation.js View 1 3 chunks +14 lines, -6 lines 0 comments Download
M app/views/topology/service.js View 4 chunks +40 lines, -0 lines 0 comments Download
M lib/views/stylesheet.less View 2 chunks +6 lines, -3 lines 0 comments Download
M test/test_environment_view.js View 1 4 chunks +83 lines, -6 lines 0 comments Download
M test/test_landscape.js View 5 chunks +9 lines, -9 lines 0 comments Download
M undocumented View 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 4
bac
Please take a look.
10 years, 11 months ago (2013-05-22 14:47:47 UTC) #1
teknico
LGTM with trivials below. Definitely an improvement in the UI, thanks. https://codereview.appspot.com/9650043/diff/1/app/store/env/simulator.js File app/store/env/simulator.js (right): ...
10 years, 11 months ago (2013-05-22 15:51:00 UTC) #2
frankban
A very nice branch Brad, thank you! This LGTM with minor questions/changes below. https://codereview.appspot.com/9650043/diff/1/app/store/env/fakebackend.js File ...
10 years, 11 months ago (2013-05-22 17:12:47 UTC) #3
bac
10 years, 11 months ago (2013-05-22 19:11:34 UTC) #4
*** Submitted:

Disable building relations if charm is not loaded.

The potential endpoints for a service are not available until the charm is
loaded.  This branch fixes a bug where an attempt was made to access the
non-existent endpoints and then blithely proceeded along.  In addition, the UI
is changed so that the 'Build Relation' submenu is disabled if the charm is
not loaded (a proxy for the endpoints not being there) and the
'long-click-to-begin-relation-building' is also disabled.

The whole relation building experience is up for redesign.  At that time we
may chose to do something that provides a better user experience, such as
showing a spinner until the service is fully available.  For now, the actual
time the actions will be disabled should be a matter of a few seconds.

A feature flag is introduced and can be used with the sandbox to artifically
set the charm loading delay for testing.  Use it as follows for a ten second
delay before the charm is reported as loaded:

http://localhost:8888/:flags:/charmLoadDelay=10000

R=teknico, frankban
CC=
https://codereview.appspot.com/9650043

https://codereview.appspot.com/9650043/diff/1/app/templates/overview.handlebars
File app/templates/overview.handlebars (right):

https://codereview.appspot.com/9650043/diff/1/app/templates/overview.handleba...
app/templates/overview.handlebars:18: 
On 2013/05/22 15:51:00, teknico wrote:
> Does this blank line have any purpose? It seems out of place.

Done.

https://codereview.appspot.com/9650043/diff/1/app/views/topology/service.js
File app/views/topology/service.js (right):

https://codereview.appspot.com/9650043/diff/1/app/views/topology/service.js#n...
app/views/topology/service.js:1210: addRelation.addClass('disabled');
It does not!  Good catch!
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b