enabled multi points gdirections

[calidion, 2010-05-04 07:34:53]

1.enabled multi points gdirections
2.updated readme

[baiming, 2010-05-05 03:31:01]

Is your demo down?

http://codereview.appspot.com/1095041/diff/2001/3001
File gtfsscheduleviewer/files/transit_editor.js (right):

http://codereview.appspot.com/1095041/diff/2001/3001#newcode1725
gtfsscheduleviewer/files/transit_editor.js:1725: * @param {Boolean} show Show
polyline or not
It seems not to be used in the function.

http://codereview.appspot.com/1095041/diff/2001/3001#newcode1729
gtfsscheduleviewer/files/transit_editor.js:1729: var VERIFY_CALLBACK_TIMEOUT =
200;
Can you comment why we need this timeout for invoking the callback?

http://codereview.appspot.com/1095041/diff/2001/3001#newcode1748
gtfsscheduleviewer/files/transit_editor.js:1748: var hint = 'Generating Shape ['
+ shape.id +
Please move the code for generating and displaying hint out of this function, to
make it pure in logic.

http://codereview.appspot.com/1095041/diff/2001/3001#newcode1752
gtfsscheduleviewer/files/transit_editor.js:1752: for (var i = 1; (i < maxPoint)
&& (start + i < self.stoptimes.length);
s/self.stoptimes.length/stopNumber

http://codereview.appspot.com/1095041/diff/2001/3001#newcode1755
gtfsscheduleviewer/files/transit_editor.js:1755: idx += 1;
Why not ++idx or idx++?

http://codereview.appspot.com/1095041/diff/2001/3001#newcode2904
gtfsscheduleviewer/files/transit_editor.js:2904: Shape.prototype.setPoints =
function(points) {
Comments are needed.

[calidion, 2010-05-05 09:33:40]

updated on comments

http://codereview.appspot.com/1095041/diff/2001/3001
File gtfsscheduleviewer/files/transit_editor.js (right):

http://codereview.appspot.com/1095041/diff/2001/3001#newcode1725
gtfsscheduleviewer/files/transit_editor.js:1725: * @param {Boolean} show Show
polyline or not
On 2010/05/05 03:31:01, baiming wrote:
> It seems not to be used in the function.

Done.

http://codereview.appspot.com/1095041/diff/2001/3001#newcode1729
gtfsscheduleviewer/files/transit_editor.js:1729: var VERIFY_CALLBACK_TIMEOUT =
200;
On 2010/05/05 03:31:01, baiming wrote:
> Can you comment why we need this timeout for invoking the callback?

Done.

http://codereview.appspot.com/1095041/diff/2001/3001#newcode1752
gtfsscheduleviewer/files/transit_editor.js:1752: for (var i = 1; (i < maxPoint)
&& (start + i < self.stoptimes.length);
On 2010/05/05 03:31:01, baiming wrote:
> s/self.stoptimes.length/stopNumber

Done.

http://codereview.appspot.com/1095041/diff/2001/3001#newcode1755
gtfsscheduleviewer/files/transit_editor.js:1755: idx += 1;
On 2010/05/05 03:31:01, baiming wrote:
> Why not ++idx or idx++?

Done.

http://codereview.appspot.com/1095041/diff/2001/3001#newcode2904
gtfsscheduleviewer/files/transit_editor.js:2904: Shape.prototype.setPoints =
function(points) {
On 2010/05/05 03:31:01, baiming wrote:
> Comments are needed.

Done.

[calidion, 2010-05-05 09:44:17]

fixed a bug.

2010/5/5 <calidion@gmail.com>

> updated on comments
>
>
>
> http://codereview.appspot.com/1095041/diff/2001/3001
> File gtfsscheduleviewer/files/transit_editor.js (right):
>
> http://codereview.appspot.com/1095041/diff/2001/3001#newcode1725
> gtfsscheduleviewer/files/transit_editor.js:1725: * @param {Boolean} show
> Show polyline or not
> On 2010/05/05 03:31:01, baiming wrote:
>
>> It seems not to be used in the function.
>>
>
> Done.
>
>
> http://codereview.appspot.com/1095041/diff/2001/3001#newcode1729
> gtfsscheduleviewer/files/transit_editor.js:1729: var
> VERIFY_CALLBACK_TIMEOUT = 200;
> On 2010/05/05 03:31:01, baiming wrote:
>
>> Can you comment why we need this timeout for invoking the callback?
>>
>
> Done.
>
>
> http://codereview.appspot.com/1095041/diff/2001/3001#newcode1752
> gtfsscheduleviewer/files/transit_editor.js:1752: for (var i = 1; (i <
> maxPoint) && (start + i < self.stoptimes.length);
> On 2010/05/05 03:31:01, baiming wrote:
>
>> s/self.stoptimes.length/stopNumber
>>
>
> Done.
>
>
> http://codereview.appspot.com/1095041/diff/2001/3001#newcode1755
> gtfsscheduleviewer/files/transit_editor.js:1755: idx += 1;
> On 2010/05/05 03:31:01, baiming wrote:
>
>> Why not ++idx or idx++?
>>
>
> Done.
>
>
> http://codereview.appspot.com/1095041/diff/2001/3001#newcode2904
> gtfsscheduleviewer/files/transit_editor.js:2904:
> Shape.prototype.setPoints = function(points) {
> On 2010/05/05 03:31:01, baiming wrote:
>
>> Comments are needed.
>>
>
> Done.
>
>
> http://codereview.appspot.com/1095041/show
>

[baiming, 2010-05-05 11:11:14]

The CL looks good to me, but found a couple of bugs in the demo.
1) Edit leg -> click on a leg -> sometimes the infowindow won't be opened, very
randomly. I don't know how to regularly reproduce it yet.
2) Edit leg -> drag the red point to get directions -> save all -> nothing is
saved. Refresh the page, then you will see the change is not taking effect.
3) Edit leg -> drag the red point to get directions -> generate shape -> the
generated shape will take over the previously modified leg, but the anchor point
is still there.

You can submit it now, and fix bugs in other CLs.

[calidion, 2010-05-06 02:04:38]

updated to rev 77

2010/5/5 <baiming@google.com>

> The CL looks good to me, but found a couple of bugs in the demo.
> 1) Edit leg -> click on a leg -> sometimes the infowindow won't be
> opened, very randomly. I don't know how to regularly reproduce it yet.
> 2) Edit leg -> drag the red point to get directions -> save all ->
> nothing is saved. Refresh the page, then you will see the change is not
> taking effect.
> 3) Edit leg -> drag the red point to get directions -> generate shape ->
> the generated shape will take over the previously modified leg, but the
> anchor point is still there.
>
> You can submit it now, and fix bugs in other CLs.
>
>
> http://codereview.appspot.com/1095041/show
>