Code review comment for lp:~acoconut/systers/flexible_essays

On Thu, Jun 28, 2012 at 3:41 AM, Ana Cutillas <email address hidden> wrote:

> On Thu, Jun 28, 2012 at 6:51 AM, Robin J <email address hidden> wrote:
>
> > OK, we are definitely making progress. Some questions and one change
> > request:
> > - what do fields 3 and 4 mean in your data structure (0, and 'bottom'
> > for the checkbox?)
> >
>
> I am not really doing anything with them yet. They are there for when I
> start adding options to make the forms beautiful and let the admin move the
> boxes around. Field 3 is the size of the box, a checkbox would look the
> same with size 3000 or 0, so we can leave it to 0. Field 4 is the relative
> position, I haven't really given it a lot of thought yet, I am currently
> printing them in the order they appear in the list. Do you think it would
> be better if I took those fields out of the data structure for now if I'm
> not using them yet?
>

No, feel free to keep them, since it doesn't change the way you interact
with the rest of the data structure. My guess is that you may need to
change some of them (probably a relative position than an absolute one),
but that can happen later.

>
> - I'm not sure what you are doing for the checkbox (because I don't
> > really understand that html). The part that confuses me is that you are
> > using question[1] there twice. One part I think you are naming the
> > checkbox, but why the second time? It seems to also be the value?
> >
> > According to what I read on the internet, there are two fields in the
> html
> syntax to add checkboxes: name and value. Name is just the name of the
> checkbox, and value is the value that is submitted if the checkbox is
> checked. That's why I'm using the same string for both.
>
> So the HTML code for a checkbox would look like this:
>
> <TD><input type='checkbox' name='woman' value='woman'></TD>
>

OK, get it now. I can't see any harm in using the same value for both of
them, but you might keep this in mind if you run into a bug -- that the
wrong field maybe is being addressed.

>
> > My requested change is that questions is too similar to mlist.questions.
> > I think the code will be much easier for someone else to read if you
> > change that to something like html_questions
> >
>
> Ok, changed! Let me know what you think about the extra 3 and 4 fields so I
> can either change that or resubmit this again.
>
> Go for it.

>
> > --
> >
> https://code.launchpad.net/~acoconut/systers/flexible_essays/+merge/112331
> > You are the owner of lp:~acoconut/systers/flexible_essays.
> >
>
> --
> https://code.launchpad.net/~acoconut/systers/flexible_essays/+merge/112331
> You are requested to review the proposed merge of
> lp:~acoconut/systers/flexible_essays into lp:systers.
>