Comments for "HTML_QuickForm_SelectFilter"

» Submit Your Comment
Comments are only accepted during the "Proposal" phase. This proposal is currently in the "Finished" phase.
» Comments
  • Gerrit Goetsch  [2004-11-12 11:31 UTC]

    Looks nice, but I prefer "HTML_QuickForm_Selectfilter" as class name, not "HTML_QuickForm_selectfilter".
    In according to the CS, please remove the tabstops and replace them with spaces.
  • Nicolas Hoizey  [2004-11-12 11:58 UTC]

    The class name uses the same convention as other QF elements. Maybe I should use the same also for the package name...

    I have removed the tabs.
  • Justin Patrin  [2004-11-12 18:47 UTC]

    There should be spaces around the '=' in the constructor declaration.

    I personally don't like heredocs. I'd rather you use ' for the string and change str == '' to str == "" within the JS.

    You have:
    would this be cleaner?
    As long as it *works*, of course, but any quotes should work.

    Again here:
    "<script type=\"text/javascript\">\n//<![CDATA[\n"
    could be:
    "<script type='text/javascript'>\n//<![CDATA[\n"
    '<script type="text/javascript">

    I prefer HTML_QuickForm_SelectFilter as the class name. Yes, it's different from the other QF elements, but it conforms to PEAR CS correctly. QF shouldn't have a problem with this class name as it doesn't do any caps changing on the class name or include filename.
  • Nicolas Hoizey  [2004-11-15 09:46 UTC]

    I've just followed coding style I found in other QF elements.

    Even if I share your point of vue, I would like to read what QF maintainers think before modifying anything.
  • Nicolas Hoizey  [2004-11-17 09:03 UTC]

    The new version now follows as strictly as possible the CS.

    The examples now use a countries list, which gives a better impact.