Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #653 (closed enhancement: fixed)

Opened 13 years ago

Last modified 11 years ago

Create a tab widget

Reported by: kevin Owned by: anonymous
Priority: normal Milestone:
Component: TG Widgets Version:
Severity: normal Keywords:
Cc: kevin

Description

There's a nice, liberally licensed tab library available:  http://www.barelyfitz.com/projects/tabber/index.php

We should consider making some kind of widget out of that.

Attachments

tabs.draft1.patch Download (18.7 KB) - added by roger.demetrescu 13 years ago.
tabs_external_files.patch Download (15.4 KB) - added by roger.demetrescu 13 years ago.
BarelyFitz?'s external files
tabs_draft_2.patch Download (6.2 KB) - added by roger.demetrescu 13 years ago.
depends on tabs_external_files.patch

Change History

comment:1 Changed 13 years ago by roger.demetrescu

  • Cc kevin added

That's my first widget, so I could be completely wrong about the TG way of doing... :)

I'm uploading a very draft widget... with some tests and a WidgetDescription? which allows it to be shown in Toolbox's Widget Browser.

I am open to any kind of suggestions... and if I am in the wrong direction please tell me...

Changed 13 years ago by roger.demetrescu

comment:2 Changed 13 years ago by alberto

Nice widget! Fast, and with tests... :) Just a few comments:

  • Shouldn't Tab be a container widget too? So you can nests tabs as the example at barelyfitz shows...
  • In Tab too, you can strip the whole init method as all it does is self.title = title, template_vars already handles that automatically (see display() @ base.Widget ).
  • Maybe MasterTab? should be Tabber? (just to keep the original semantics)
  • We've somehow informally decided that the only positional arguments that widget's should receive is the "name" parameter as it's the most frequently used one, ie: Form(fields=[TextField?("name"), TextField?("age")]), maybe the "title" param at Tab should follow this convention too.

Gonna give it a closer look when I get to work as I'm working on a gui now and this looks very promising...

Thanks, Great work! :)

Alberto

comment:3 Changed 13 years ago by roger.demetrescu

  • Cc kevin removed

Thanks Alberto for all your comments.

Initially I thought about using Tabber/Tab? for the widgets names. But with the idea of abstracting the actual JS implementation, I switched the "Tabber" name to "MasterTab?" (or could be "MainTab?"). But I'll follow your suggestion of turning back to "Tabber". :)

I'll make a 2nd round on this widget tonight... :)

Thanks again.

Roger

PS: I'll cleanup the CC: to not bore Kevin while this is not ready.

Changed 13 years ago by roger.demetrescu

BarelyFitz?'s external files

Changed 13 years ago by roger.demetrescu

depends on tabs_external_files.patch

comment:4 Changed 13 years ago by roger.demetrescu

I'm releasing the 2nd draft of the Tabber widget. There are some small issues to solve, like the necessity of including "media" attribute for the CSS declarations (see  http://tinyurl.com/ft6xm).

I'd love to appreciate your feedback.

Cheers Roger

comment:5 Changed 13 years ago by alberto

Hi Roger,

This 2nd draft looks great IMO. Only thing left is the handling of normal text as tabcontent, right? I think Michele's Idea of wrapping it in a DumpString?, or StringContent? (or something) widget is the way to go...

I'm pinging Kevin so he can take a look at it for the final veredict...

Great work!, Thanks

Alberto

P.S Im going to add a "media" template_var to the CSS* widgets. It should allow you to CSSLink(..., media="screen"), I think this should be enough, but let me know if I miss something as I've not followed very closely the discussions lately :(

comment:6 Changed 13 years ago by alberto

Since r919 CSSLink and CSSSource have a media attribute, is this what you need? Alberto

comment:7 Changed 13 years ago by anonymous

  • Cc kevin added

comment:8 Changed 13 years ago by roger.demetrescu

Hi Alberto,

You're right.. adding a "media" attribute is enough for me. :)

I feel that handling normal text would be a nice feature... but as I am/was not sure it it would follow TG's way of doing things, I preferred to ask you guys before.

BTW, I lost a big time figuring out why my "content" property in Tab class was causing errors, just to find out that "content" was a reserved word for Kid. Maybe having a Widget Developer's guide in the future would help other people in the future...

Thank you and Michelle for some tips. :)

Roger

comment:9 Changed 13 years ago by alberto

class StringContent(Widget):
    template_vars = ["str_content"]
    template = """<div py:replace="str_content" /> """

def update_data(.....):
    ...
    if isinstance(d["tab_content"], basestring):
        d["tab_content"] = StringContent(str_content=d["tab_content"])
    ...

I *think* this should be enough (beware, untested, might even trash your hard drive ;)

Reagarding the content property, I wasn't aware of this... you wont go to bed without learning new things :)

Regarding the docs, you're absolutely right! When #613 is done and the API finally settles down maybe we (all the widget developers) should sit together and write a "Widget's Doc & Best-Practices" page... till then, the Widget sources should fill this gap, that's why we're so picky about the style ;)

Thanks again, Alberto

comment:10 Changed 13 years ago by roger.demetrescu

Hi Alberto...

Should StringContent? class be in widget.py (as a public utility) or in tabs.py (as an internal wrapper, not available in all = [...]) ?

Thanks !

comment:11 Changed 13 years ago by roger.demetrescu

Not to me: ALWAYS preview your writings because Wiki can mess some symbols !!

Where you read "all = [...]" it should be:

# yeap, you won't have StringContent... it belongs to me !!!
__all__ = ["Tabber", "Tab"]

comment:12 Changed 13 years ago by alberto

I think it won't do harm to have it in base.py, it's very general and could be of use in the future, though we can still move it there when the need arises... Dunno... +1 for base.py for the time being, but don't take me too seriously on this ;)

Alberto

comment:13 Changed 13 years ago by roger.demetrescu

Alberto, I'm still having problems with CSSLink...

With your patch I can get it:

>>> css = CSSLink(mod=static, name="sample.css")
>>> css.render()
'<LINK MEDIA="all" HREF="/tg_widgets/turbogears.widgets/sample.css" TYPE="text/css" REL="stylesheet">'
>>> css.render(media="printer")
'<LINK MEDIA="printer" HREF="/tg_widgets/turbogears.widgets/sample.css" TYPE="text/css" REL="stylesheet">'

That is fine. But what I wanted to do is this:

>>> css = CSSLink(mod=static, name="sample.css", media="printer")

Traceback (most recent call last):
  File "<pyshell#45>", line 1, in -toplevel-
    css = CSSLink(mod=static, name="sample.css", media="printer")
  File "D:\Turbogears\turbogears\widgets\base.py", line 50, in widgetinit
    func(self, *args, **kw)
TypeError: __init__() got an unexpected keyword argument 'media'

So media attribute can't be used in the constructor, just in render().

Maybe I am doing something wrong, but may initial intention was to use the CSSLink this way:

class Tabber(CompoundWidget):
    css = [CSSLink(mod=static, name="tabber.css", media="screen"),
           CSSLink(mod=static, name="tabber-print.css", media="printer")]
    javascript = [JSLink(mod=static, name="tabber.js")]
    template = """
    <div xmlns:py="http://purl.org/kid/ns#" id="${name}" class="tabber">
        <div py:for="tab,defaulttab in tabs" py:replace='tab.display(defaulttab=defaulttab)' />
    </div>
    """
    # continue ...

But as you can imagine, it will cause an error...

AFAIK, CSSLink & CSSSource should be fixed to allow media attribute in init().

Thanks again!

comment:14 Changed 13 years ago by alberto

take a look at r921... thanks for spotting this ;) Alberto

comment:15 Changed 13 years ago by alberto

errr, r922, forgot to the the same to CSSSource

comment:16 Changed 13 years ago by roger.demetrescu

Thanks Alberto... it works perfectly now. :)

I'll try to release the 3th (and hopelly the ready to go to trunk) version tonight. Yesterday I couldn't work on this because in the last 48 hours I just slept 8 hours... so I was afraid of messing things here...

Now I am fine, don't worry... :)

Thanks again ! Roger

comment:17 Changed 13 years ago by roger.demetrescu

Ok... if you are following this ticket and missing some ML posts, you should really take a look at this thread:  http://tinyurl.com/fux5m

I understand and agree that my patches are not the way to go. The sentence that I think that best summarizes what is expected from this widget is quoted from a Kevin's message:

"I'd go as far as saying that a widget that just provides layout should
not go into the TurboGears core. (People are always free to make their
own widget packages...)

I would not be opposed to a widget that brings a JavaScript library in
and applies markup at the JavaScript level (eg SyntaxHighlighter,
which changes every textarea with a name="code" to a highlighted
block)"

So this task is completely opened again. Beside CSSLink and JSLink declarations, no other code should be taken in account when designing a new widget... It really should be developed from scratch... I'm warning you... :D

Anyway, I really want to thanks Alberto and Michelle for being so helpfull... I've learned a lot studying the widgets source code, and your help have clarified some questions...

See you... :D

comment:18 Changed 13 years ago by alberto

  • Status changed from new to closed
  • Resolution set to fixed

Committed a Tabber at [1416] Thanks Rooger for your initial attempt! :) Alberto

comment:19 Changed 13 years ago by roger.demetrescu

Cool !!

Thank you... :)

Note: See TracTickets for help on using tickets.