Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

http://codereview.appspot.com/download/issue1701046_1.diff #291

Open
pyjsorg opened this Issue · 3 comments

1 participant

@pyjsorg
Owner

series of patches by serge (good stuff).

Original issue: http://code.google.com/p/pyjamas/issues/detail?id=428 (June 23, 2010 15:20:32)

@pyjsorg
Owner

From luke.lei...@gmail.com on June 23, 2010 15:31:04:
(this is duplicated at http://code.google.com/p/pyjamas/issues/detail?id=428 - please use that to discuss this issue)

On Jun 23, 3:43 pm, Serge Tarkovski serge.tarkov...@gmail.com wrote:

Hi all!

allo :)

I've implemented some patches - could anyone take a look?

yyyup. best working practice / preference: add to http://code.google.com/p/pyjamas/issues as an attachment, then we go from there.

CaptionPanel - added default style

yes. good. why wasn't that there already?

DialogBox - added possibility to be auto-centerable on show and on
window resize, added optional 'close' button

like the principle. not so keen on the gif file.

MenuBar - additional styles for vertical and horizontal menu to
distinguish them visually (as GWT does)

eexcellent.

  • def getDefaultStyleName(self):
  •    return "gwt-MenuBar " + "gwt-MenuBar-vertical" if self.vertical else "gwt-MenuBar-horizontal"
    

    this is over 80 chars. not ok.

  • self.popup.setPopupPosition(self.getAbsoluteLeft() +
  • self.popup.setPopupPosition(self.getAbsoluteLeft() +

i preferrr these as separate patches (labelled "whitespace cleanup"). git-am will help you to do multi-patches.

TabPanel - added possibility to create closable tabs

i can't see clearly what's been done here, because you moved the function. please don't do that and make code changes.

PopupPanel

ah. you've now created enableGlass and disableGlass, which puts this firmly into the "setXxxxx" category. please can you therefore:

  • rename glass= to Glass= in the constructor
  • create a setGlass function which takes on param (True/False)
  • remove the hard-coded setup of self.glass = DOM.createDiv() etc.
  • store the GlassStyleName parameter as a member in the class
  • use that parameter in setGlass, calling self.setGlassStyleName(self.glassStyleName)

    if this is too complex / not obvious, i'll do it.

The code is here -http://codereview.appspot.com/1701046(please leave
comments there)

nooo, please disregard this request.

comments should be left in the issue tracker, where they belong, so that people can track them in the correct place.

Examples will come later.

excellent. as attachments / patches, added to the issue tracker, please.

Also, I've been playing with pyjsglade tool and, being unsatisfied
with its default styling, I have stolen CSS from GWT examples page,
and applied it with some minor modifications. Some screenshots could
be seen here -http://imagebin.ca/view/0veCdfZ.htmlhttp://imagebin.ca/view/hBstObBO.htmlhttp://imagebin.ca/view/KDWH0f.html

If anyone wants, I could provide current CSS I use - I am not
publishing it only due it has a lot of stuff not applicable to Pyjamas
at the moment and requires a refactoring.

i'm sure kees would be interested.

Would you guys consider adding some default styling bundled with
Pyjamas distrubution?

yes. it's been on the TODO list for a while (issue tracker). it would be really nice to have some sort of auto-generator of stylesheet fragments which can automatically be added even under pyjamas-desktop.

that might have to involve some code which creates a node and adds it to the DOM <em>but</em> adds it early enough so that manual CSS stylesheets can override the entries.</p> <p>hmm, i wonder if that will even fly? :)</p> <p>l.</p>

@pyjsorg
Owner

From luke.lei...@gmail.com on June 23, 2010 17:14:22:
ok, ok, they're all .gifs in the pyjs/public directory grumble, so a few more can't hurt. could you attach them here as actual file attachments because they're binary files and "git apply" won't accept the appspot-generated patch.

@pyjsorg
Owner

From luke.lei...@gmail.com on June 23, 2010 17:44:34:
ok some of these are in. makeCloseable i renamed to setCloseable and it is now called through UIApplier, by setting "Closeable=True" not "closeable=True".

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.