Skip to content

Update DisclosurePanel to be cleaner, more versatile, and more featureful. #330

Open
pyjsorg opened this Issue Apr 27, 2012 · 7 comments

1 participant

@pyjsorg
pyjsorg commented Apr 27, 2012

The attached patch is an enhancement to the existing DisclosurePanel
widget. The API has not changed except for DisclosurePanel.getOpen is now
DisclosurePanel.isOpen, in line with the GWT.

The DefaultHeader and ClickableHeader classes have been merged into the
DefaultHeader class, for simplicity, and I don't think this decreases
versatility. Also the builtin ClickHandler API is used instead of custom
event handling, which allows for more versatility.

Original issue: http://code.google.com/p/pyjamas/issues/detail?id=389 (April 04, 2010 17:39:25)

@pyjsorg
pyjsorg commented Apr 27, 2012

From luke.lei...@gmail.com on April 08, 2010 18:24:55:
ok - lots of things.

1) sadly i do not believe that we can remove ClickableHeader: it has a purpose (see
comments in GWT java source code). however, making ClickableHeader derive from
ClickHandler would be good.

2) SVGImage does not have anything to do with DisclosurePanel. it therefore needs to
be a separate patch, separate bugreport, separate CHANGELOG patch entry (i recommend
you begin using git-svn and then use the git am command if you're going to be doing
multiple patches like this)

3) i'm not keen on the renaming of isOpen to something as short and generic as the
word "open". much as i find pseudoCanonicalisation annoying, i'm much more
comfortable with isOpen than with "open". is_open would be better; _is_open even
better, but just "open" .... naah.

4) we cannot put in commented-out "# print" debug statements into live-running
code... :)

5) even when quite small, the general rule is that new classes go into their own
separate module. thus, SVGImage should go into pyjamas/ui/SVGImage.py not
pyjamas/ui/Image.py

@pyjsorg
pyjsorg commented Apr 27, 2012

From luke.lei...@gmail.com on April 08, 2010 18:30:30:
6) remember to add an entry into the CHANGELOG - one that describes what's been done
for this issue and exclusively for this issue - i.e. excluding the SVGImage
addition: that needs to also have its own CHANGELOG entry.

@pyjsorg
pyjsorg commented Apr 27, 2012

From glenn.wa...@gmail.com on April 09, 2010 20:52:05:
1) Yep, looks like you're right, I hadn't looked at the GWT code and hadn't
considered that it would be important. I agree with your rationale given on the
mailing list. Given that, wouldn't making ClickableHeader derive from ClickHandler
also violate this property?

2 & 4) Oops, I included the wrong patch. It was only supposed to be
library/pyjamas/ui/DisclosurePanel.py. SVGImage currently doesn't work for FF, if I
ever get it working I'll submit it as a separate issue.

3) Sounds reasonable, I can change it to _is_open. Basically, I felt I needed to
rename it to allow isOpen to be a method to conform to the GWT API.

5) Noted.

@pyjsorg
pyjsorg commented Apr 27, 2012

From luke.lei...@gmail.com on April 25, 2010 12:49:41:
3 sounds like a good idea.

1 - don't worry about it :) we can't have absolutely everything. ClickHandler,
KeyboardHandler etc. is one of theee very rare places where pyjamas deviates from gwt,
and given that python doesn't support the concept of "interfaces", thus we have to
do something different, i don't see this as a problem.

@pyjsorg
pyjsorg commented Apr 27, 2012

From luke.lei...@gmail.com on June 08, 2010 14:13:17:
how you getting on with this, glenn?

@pyjsorg
pyjsorg commented Apr 27, 2012

From luke.lei...@gmail.com on June 24, 2010 23:55:27:
ok y'know what? yes :)

@pyjsorg
pyjsorg commented Apr 27, 2012

From luke.lei...@gmail.com on June 24, 2010 23:58:05:
whoops sorry closed wrong bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.