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

Issue 7177050: Fix ambiguous relation selector

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 3 months ago by frankban
Modified:
8 years, 3 months ago
Reviewers:
mp+144301
Visibility:
Public.

Description

Fix ambiguous relation selector When there are ambiguous relations clicking to select one or the other does nothing. This bug affected only our production server, devel and debug worked well. This is weird, and I think we are encountering the strange behavior described in http://yuilibrary.com/projects/yui3/ticket/2532993 or similar: in some situation (e.g. our production server, where js files are minified/compressed) using the "on" method on NodeList does not work. I am confused about the real reason for this bug. However, the documentation says that we should avoid using NodeList.on, in favor of using event delegation, that is exactly what I did in this branch. Tests are not included: we have a specific card for adding tests for the relation module. https://code.launchpad.net/~frankban/juju-gui/bug-1102638-ambigous-relation/+merge/144301 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 2

Patch Set 2 : Fix ambiguous relation selector #

Patch Set 3 : Fix ambiguous relation selector #

Unified diffs Side-by-side diffs Delta from patch set Stats (+18 lines, -16 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M app/views/topology/relation.js View 1 1 chunk +15 lines, -15 lines 0 comments Download
M test/test_topology.js View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 8
frankban
Please take a look.
8 years, 3 months ago (2013-01-22 12:46:15 UTC) #1
frankban
On 2013/01/22 12:46:15, frankban wrote: > Please take a look. Mentioned YUI documentation can be ...
8 years, 3 months ago (2013-01-22 12:47:17 UTC) #2
benji
Looks good.
8 years, 3 months ago (2013-01-22 13:32:45 UTC) #3
gary.poster
Great catch, Francesco, thank you. Land with changes. Gary https://codereview.appspot.com/7177050/diff/1/app/views/topology/relation.js File app/views/topology/relation.js (right): https://codereview.appspot.com/7177050/diff/1/app/views/topology/relation.js#newcode640 app/views/topology/relation.js:640: ...
8 years, 3 months ago (2013-01-22 13:58:20 UTC) #4
frankban
Thanks for the reviews. Added to this branch a trivial fix for a test failing ...
8 years, 3 months ago (2013-01-22 14:24:53 UTC) #5
frankban
Thanks for the reviews. Added to this branch a trivial fix for a test failing ...
8 years, 3 months ago (2013-01-22 14:25:04 UTC) #6
frankban
Please take a look.
8 years, 3 months ago (2013-01-22 14:33:47 UTC) #7
frankban
8 years, 3 months ago (2013-01-22 14:38:12 UTC) #8
*** Submitted:

Fix ambiguous relation selector

When there are ambiguous relations clicking 
to select one or the other does nothing.
This bug affected only our production server,
devel and debug worked well. This is weird,
and I think we are encountering the strange
behavior described in 
http://yuilibrary.com/projects/yui3/ticket/2532993
or similar: in some situation (e.g. our production
server, where js files are minified/compressed)
using the "on" method on NodeList does not work.
I am confused about the real reason for this bug.
However, the documentation says that we should
avoid using NodeList.on, in favor of using event 
delegation, that is exactly what I did in this
branch. Tests are not included: we have a specific 
card for adding tests for the relation module.

R=benji, gary.poster
CC=
https://codereview.appspot.com/7177050
Sign in to reply to this message.

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