enabled Stop Info on Click

[calidion, 2010-01-22 03:36:28]

1. removed stop.js
2. removed useless spaces
3. enabled stop marker click handler

[calidion, 2010-01-22 04:49:05]

preview:
http://211.100.227.25:8765/

2010/1/22 <calidion@gmail.com>

> Reviewers: lychen, xan_google.com, weiliu, leio.chen, baiming, qiaojian,
>
> Message:
> 1. removed stop.js
> 2. removed useless spaces
> 3. enabled stop marker click handler
>
>
>
> Please review this at http://codereview.appspot.com/193049/show
>
> Affected files:
>  M     gtfsscheduleviewer/files/index.html
>  D     gtfsscheduleviewer/files/stop.js
>  M     gtfsscheduleviewer/files/transit_editor.js
>  M     schedule_editor.py
>  M     transitfeed_editor.py
>
>
>

[baiming, 2010-01-22 06:20:59]

http://codereview.appspot.com/193049/diff/1/3
File gtfsscheduleviewer/files/transit_editor.js (right):

http://codereview.appspot.com/193049/diff/1/3#newcode2
gtfsscheduleviewer/files/transit_editor.js:2: function parseTimeInput() {
Add jsDoc.

How about renaming it to parseTimeInputToSeconds?

http://codereview.appspot.com/193049/diff/1/3#newcode4
gtfsscheduleviewer/files/transit_editor.js:4: var m =
text.match(/([012]?\d):([012345]?\d)(:([012345]?\d))?/);
1) What if a user types 27:00:00 in the box? can we have a more restricted check
on that?
2) Do we have a hint or an example to tell users what format of time to type?
3) Why not using [0-5] to replace [012345]?
3) Please use non-capturing group for the current m[3], which is not needed to
be captured. More specific, replace (:([012345]?\d))? with (?::([0-5]?d))?
By doing this, m[4] becomes m[3].

http://codereview.appspot.com/193049/diff/1/3#newcode1602
gtfsscheduleviewer/files/transit_editor.js:1602: this.info = data;
What actual information does "data/this.info" contain? Please give it a simple
comment, and also describe the data structure of it.

http://codereview.appspot.com/193049/diff/1/3#newcode1610
gtfsscheduleviewer/files/transit_editor.js:1610: Stop.prototype.showInfo =
function() {
Please reorganize the flow of this function. It is a little hard to read now.

Stop.prototype.showInfo = function() {
  var timeTrips = this.info;

  // Format the title part (name and lat/lng)
  var titleDiv = $C('div');
  titleDiv.innerHTML = ...;
  ...

  // Format the trips table
  var tripsTable = $C('table');

  // Format the header of trips table
  var headerRow = $C('tr');
  headerRow.appendChild(...
  ...
  tripsTable.appendChild(headerRow);

  // Format the body of trips table
  for (var i = 0; i < timeTrips.length; ++i) {
    var tripRow = $('C');
    ...
    tripsTalbe.appendChild(tripRow);
  }

  // Insert all sections to a div
  var div = $('div');
  div.appendChild(titleDiv);
  div.appendChild(tripsTable);

  // Open infowindow
  map.openInfoWindow(this.latlng, div);
};

http://codereview.appspot.com/193049/diff/1/3#newcode1621
gtfsscheduleviewer/files/transit_editor.js:1621: th.style.fontWeight = 'bold';
Can you use css classes to control the styles, rather than setting them in js
code?

[weiliu, 2010-01-22 07:15:29]

Most of the python changes are cleaning the unnecessary spaces, right?

http://codereview.appspot.com/193049/diff/1/5
File transitfeed_editor.py (right):

http://codereview.appspot.com/193049/diff/1/5#newcode396
transitfeed_editor.py:396: If both are None return None."""
Please follow the python style guide for comments:
http://code.google.com/p/soc/wiki/PythonStyleGuide#Functions_and_Methods

[calidion, 2010-01-22 07:23:46]

updated

http://codereview.appspot.com/193049/diff/1/3
File gtfsscheduleviewer/files/transit_editor.js (right):

http://codereview.appspot.com/193049/diff/1/3#newcode2
gtfsscheduleviewer/files/transit_editor.js:2: function parseTimeInput() {
On 2010/01/22 06:21:00, baiming wrote:
> Add jsDoc.
> 
> How about renaming it to parseTimeInputToSeconds?

Removed.

http://codereview.appspot.com/193049/diff/1/3#newcode4
gtfsscheduleviewer/files/transit_editor.js:4: var m =
text.match(/([012]?\d):([012345]?\d)(:([012345]?\d))?/);
On 2010/01/22 06:21:00, baiming wrote:
> 1) What if a user types 27:00:00 in the box? can we have a more restricted
check
> on that?
> 2) Do we have a hint or an example to tell users what format of time to type?
> 3) Why not using [0-5] to replace [012345]?
> 3) Please use non-capturing group for the current m[3], which is not needed to
> be captured. More specific, replace (:([012345]?\d))? with (?::([0-5]?d))?
> By doing this, m[4] becomes m[3].

it is copied from schedule viewer, and seems of no importance to use it here.
i would like to remove this function

http://codereview.appspot.com/193049/diff/1/3#newcode1602
gtfsscheduleviewer/files/transit_editor.js:1602: this.info = data;
On 2010/01/22 06:21:00, baiming wrote:
> What actual information does "data/this.info" contain? Please give it a simple
> comment, and also describe the data structure of it.

Done.

http://codereview.appspot.com/193049/diff/1/3#newcode1610
gtfsscheduleviewer/files/transit_editor.js:1610: Stop.prototype.showInfo =
function() {
On 2010/01/22 06:21:00, baiming wrote:
> Please reorganize the flow of this function. It is a little hard to read now.
> 
> Stop.prototype.showInfo = function() {
>   var timeTrips = this.info;
> 
>   // Format the title part (name and lat/lng)
>   var titleDiv = $C('div');
>   titleDiv.innerHTML = ...;
>   ...
> 
>   // Format the trips table
>   var tripsTable = $C('table');
> 
>   // Format the header of trips table
>   var headerRow = $C('tr');
>   headerRow.appendChild(...
>   ...
>   tripsTable.appendChild(headerRow);
> 
>   // Format the body of trips table
>   for (var i = 0; i < timeTrips.length; ++i) {
>     var tripRow = $('C');
>     ...
>     tripsTalbe.appendChild(tripRow);
>   }
> 
>   // Insert all sections to a div
>   var div = $('div');
>   div.appendChild(titleDiv);
>   div.appendChild(tripsTable);
> 
>   // Open infowindow
>   map.openInfoWindow(this.latlng, div);
> };

Done.

http://codereview.appspot.com/193049/diff/1/3#newcode1621
gtfsscheduleviewer/files/transit_editor.js:1621: th.style.fontWeight = 'bold';
On 2010/01/22 06:21:00, baiming wrote:
> Can you use css classes to control the styles, rather than setting them in js
> code?
removed

[baiming, 2010-01-22 07:26:36]

LGTM

http://codereview.appspot.com/193049/diff/17/1003
File gtfsscheduleviewer/files/transit_editor.js (right):

http://codereview.appspot.com/193049/diff/17/1003#newcode245
gtfsscheduleviewer/files/transit_editor.js:245: + "&time=0";
What's the purpose of parameter "time"? If is of no use any longer, please
remove it from the generated url as well.

[calidion, 2010-01-22 07:38:18]

it seems that it is used to find the next trips of a maximumly count 5 which
we can have according to the time input.
And i think it is confusing to the end users and prefer not to use it.

Currently the server side will still read the parameter 'time'.
default 0 is for all the trips we can fetch.
so we shall keep it now.

2010/1/22 <baiming@google.com>

> LGTM
>
>
> http://codereview.appspot.com/193049/diff/17/1003
>
> File gtfsscheduleviewer/files/transit_editor.js (right):
>
> http://codereview.appspot.com/193049/diff/17/1003#newcode245
> gtfsscheduleviewer/files/transit_editor.js:245: + "&time=0";
> What's the purpose of parameter "time"? If is of no use any longer,
> please remove it from the generated url as well.
>
>
> http://codereview.appspot.com/193049/show
>

[calidion, 2010-01-22 07:48:29]

updated.

http://codereview.appspot.com/193049/diff/1/5
File transitfeed_editor.py (right):

http://codereview.appspot.com/193049/diff/1/5#newcode396
transitfeed_editor.py:396: If both are None return None."""
On 2010/01/22 07:15:29, weiliu wrote:
> Please follow the python style guide for comments:
> http://code.google.com/p/soc/wiki/PythonStyleGuide#Functions_and_Methods

Done.

[calidion, 2010-01-22 07:51:19]

yes.

2010/1/22 <weiliu@google.com>

> Most of the python changes are cleaning the unnecessary spaces, right?
>
>
> http://codereview.appspot.com/193049/diff/1/5
> File transitfeed_editor.py (right):
>
> http://codereview.appspot.com/193049/diff/1/5#newcode396
> transitfeed_editor.py:396: If both are None return None."""
> Please follow the python style guide for comments:
> http://code.google.com/p/soc/wiki/PythonStyleGuide#Functions_and_Methods
>
>
> http://codereview.appspot.com/193049/show
>

[weiliu, 2010-01-22 07:53:33]

LGTM.

2010/1/22 李白，字一日 <calidion@gmail.com>

> yes.
>
> 2010/1/22 <weiliu@google.com>
>
> Most of the python changes are cleaning the unnecessary spaces, right?
>>
>>
>> http://codereview.appspot.com/193049/diff/1/5
>> File transitfeed_editor.py (right):
>>
>> http://codereview.appspot.com/193049/diff/1/5#newcode396
>> transitfeed_editor.py:396: If both are None return None."""
>> Please follow the python style guide for comments:
>> http://code.google.com/p/soc/wiki/PythonStyleGuide#Functions_and_Methods
>>
>>
>> http://codereview.appspot.com/193049/show
>>
>
>


-- 
Best Regards,
Wei Liu
86-10-62503256(o)

[baiming, 2010-01-22 07:55:38]

LGTM

OK. Fine with me.

2010/1/22 李白，字一日 <calidion@gmail.com>:
> it seems that it is used to find the next trips of a maximumly count 5 which
> we can have according to the time input.
> And i think it is confusing to the end users and prefer not to use it.
> Currently the server side will still read the parameter 'time'.
> default 0 is for all the trips we can fetch.
> so we shall keep it now.
>
> 2010/1/22 <baiming@google.com>
>>
>> LGTM
>>
>>
>> http://codereview.appspot.com/193049/diff/17/1003
>> File gtfsscheduleviewer/files/transit_editor.js (right):
>>
>> http://codereview.appspot.com/193049/diff/17/1003#newcode245
>> gtfsscheduleviewer/files/transit_editor.js:245: + "&time=0";
>> What's the purpose of parameter "time"? If is of no use any longer,
>> please remove it from the generated url as well.
>>
>> http://codereview.appspot.com/193049/show
>
>



-- 
Ming Bai 白明
Software Engineer
Google Inc.
Tel(Office): 86 (10) 6250-3361
Tel(Cell): +86-159-0153-4908
Email: baiming@google.com

[calidion, 2010-01-22 08:18:52]

updated to rev 49.

hi all,

we have finished the import of transit viewer, and restructured it in a way
from function powered to object oriented.

And Tons of thanks to all of you, especially to biaming, lychen, weiliu,
whose suggestions help a lot in how to make the code more professional, more
readable and better structured.


Thanks,
Wenxin




2010/1/22 Ming Bai 白明 <baiming@google.com>

> LGTM
>
> OK. Fine with me.
>
> 2010/1/22 李白，字一日 <calidion@gmail.com>:
> > it seems that it is used to find the next trips of a maximumly count 5
> which
> > we can have according to the time input.
> > And i think it is confusing to the end users and prefer not to use it.
> > Currently the server side will still read the parameter 'time'.
> > default 0 is for all the trips we can fetch.
> > so we shall keep it now.
> >
> > 2010/1/22 <baiming@google.com>
> >>
> >> LGTM
> >>
> >>
> >> http://codereview.appspot.com/193049/diff/17/1003
> >> File gtfsscheduleviewer/files/transit_editor.js (right):
> >>
> >> http://codereview.appspot.com/193049/diff/17/1003#newcode245
> >> gtfsscheduleviewer/files/transit_editor.js:245: + "&time=0";
> >> What's the purpose of parameter "time"? If is of no use any longer,
> >> please remove it from the generated url as well.
> >>
> >> http://codereview.appspot.com/193049/show
> >
> >
>
>
>
> --
> Ming Bai 白明
> Software Engineer
> Google Inc.
> Tel(Office): 86 (10) 6250-3361
> Tel(Cell): +86-159-0153-4908
> Email: baiming@google.com
>

[weiliu, 2010-01-22 08:21:37]

Great, thanks for your hard work!

2010/1/22 李白，字一日 <calidion@gmail.com>

> updated to rev 49.
>
> hi all,
>
> we have finished the import of transit viewer, and restructured it in a way
> from function powered to object oriented.
>

Our next step is to start the part for the editor, right? Would you please
list items in your plan?

>
>
And Tons of thanks to all of you, especially to biaming, lychen, weiliu,
> whose suggestions help a lot in how to make the code more professional, more
> readable and better structured.
>
>
> Thanks,
> Wenxin
>
>
>
>
> 2010/1/22 Ming Bai 白明 <baiming@google.com>
>
> LGTM
>>
>> OK. Fine with me.
>>
>> 2010/1/22 李白，字一日 <calidion@gmail.com>:
>> > it seems that it is used to find the next trips of a maximumly count 5
>> which
>> > we can have according to the time input.
>> > And i think it is confusing to the end users and prefer not to use it.
>> > Currently the server side will still read the parameter 'time'.
>> > default 0 is for all the trips we can fetch.
>> > so we shall keep it now.
>> >
>> > 2010/1/22 <baiming@google.com>
>> >>
>> >> LGTM
>> >>
>> >>
>> >> http://codereview.appspot.com/193049/diff/17/1003
>> >> File gtfsscheduleviewer/files/transit_editor.js (right):
>> >>
>> >> http://codereview.appspot.com/193049/diff/17/1003#newcode245
>> >> gtfsscheduleviewer/files/transit_editor.js:245: + "&time=0";
>> >> What's the purpose of parameter "time"? If is of no use any longer,
>> >> please remove it from the generated url as well.
>> >>
>> >> http://codereview.appspot.com/193049/show
>> >
>> >
>>
>>
>>
>> --
>> Ming Bai 白明
>> Software Engineer
>> Google Inc.
>> Tel(Office): 86 (10) 6250-3361
>> Tel(Cell): +86-159-0153-4908
>> Email: baiming@google.com
>>
>
>


-- 
Best Regards,
Wei Liu
86-10-62503256(o)

[calidion, 2010-01-22 08:23:24]

ok, I will send it later.

2010/1/22 Wei Liu <weiliu@google.com>

> Great, thanks for your hard work!
>
> 2010/1/22 李白，字一日 <calidion@gmail.com>
>
>> updated to rev 49.
>>
>> hi all,
>>
>> we have finished the import of transit viewer, and restructured it in a
>> way from function powered to object oriented.
>>
>
> Our next step is to start the part for the editor, right? Would you please
> list items in your plan?
>
>>
>>
> And Tons of thanks to all of you, especially to biaming, lychen, weiliu,
>> whose suggestions help a lot in how to make the code more professional, more
>> readable and better structured.
>>
>>
>> Thanks,
>> Wenxin
>>
>>
>>
>>
>> 2010/1/22 Ming Bai 白明 <baiming@google.com>
>>
>> LGTM
>>>
>>> OK. Fine with me.
>>>
>>> 2010/1/22 李白，字一日 <calidion@gmail.com>:
>>> > it seems that it is used to find the next trips of a maximumly count 5
>>> which
>>> > we can have according to the time input.
>>> > And i think it is confusing to the end users and prefer not to use it.
>>> > Currently the server side will still read the parameter 'time'.
>>> > default 0 is for all the trips we can fetch.
>>> > so we shall keep it now.
>>> >
>>> > 2010/1/22 <baiming@google.com>
>>> >>
>>> >> LGTM
>>> >>
>>> >>
>>> >> http://codereview.appspot.com/193049/diff/17/1003
>>> >> File gtfsscheduleviewer/files/transit_editor.js (right):
>>> >>
>>> >> http://codereview.appspot.com/193049/diff/17/1003#newcode245
>>> >> gtfsscheduleviewer/files/transit_editor.js:245: + "&time=0";
>>> >> What's the purpose of parameter "time"? If is of no use any longer,
>>> >> please remove it from the generated url as well.
>>> >>
>>> >> http://codereview.appspot.com/193049/show
>>> >
>>> >
>>>
>>>
>>>
>>> --
>>> Ming Bai 白明
>>> Software Engineer
>>> Google Inc.
>>> Tel(Office): 86 (10) 6250-3361
>>> Tel(Cell): +86-159-0153-4908
>>> Email: baiming@google.com
>>>
>>
>>
>
>
> --
> Best Regards,
> Wei Liu
> 86-10-62503256(o)
>