[gwt-contrib] ** PROPOSED API ADDITION ** -- MenuItemSeparator

[John LaBanca]

This proposal is to add a MenuItemSeparator class that can be added to a MenuBar, creating a thin line (customizable via CSS) that separates groups of MenuItems.

See the thread " Code Review Request - Fix for #1917 - Allow non-selectable menuitems (seperators)" for the patch:
http://groups.google.com/group/Google-Web-Toolkit-Contributors/browse_thread/thread/67d9f19d53273e8f

Separators are a common feature available in most menu bars, and they can be very useful in grouping MenuItems.  In addition to the new MenuItemSeparator class, a few methods will be added to MenuBar:

/**
 * Adds a thin line to the {@link MenuBar} to separate sections of
 * {@link MenuItem}s.
 */
public MenuItemSeparator MenuBar.addSeparator();

/**
 * Adds a thin line to the {@link MenuBar} to separate sections of
 * {@link MenuItem}s.
 */
public void MenuBar.addSeparator(MenuItemSeparator);  // Adds a separator, which can be a specialized subclass

/**
 * Removes the specified {@link MenuItemSeparator} from the bar.
 */
public void MenuBar.removeSeparator(MenuItemSeparator);  // Removes a separator that is already in the MenuBar

The MenuItemSeparator is not a subclass of MenuItem, so it does not have an associated Command or sub MenuBar.  In addition, Separators are not selectable or highlightable in the MenuBar.

Any objections?

--
Thanks,
John LaBanca
jlab...@google.com

[Ian Petersen]

I think this method:

/**
* Adds a thin line to the {@link MenuBar} to separate sections of
* {@link MenuItem}s.
*/
public void MenuBar.addSeparator(MenuItemSeparator); // Adds a
separator, which can be a specialized subclass

Should return the passed-in MenuItemSeparator to mirror this method

/**
* Adds a thin line to the {@link MenuBar} to separate sections of
* {@link MenuItem}s.
*/
public MenuItemSeparator MenuBar.addSeparator();

and to support this kind of code:

MenuItemSeparator separator = aMenuBar.addSeparator(new
SpecializedMenuItemSeparator());

which would provide symmetry with this kind of code:

MenuItemSeparator separator = aMenuBar.addSeparator();

Ian

--
Tired of pop-ups, security holes, and spyware?
Try Firefox: http://www.getfirefox.com

[John LaBanca]

That would be inconsistent with MenuBar.addItem(MenuItem), which returns void.  However, I'm not personally against it if the benefit outweighs the inconsistency.

[Ian Petersen]

On Jan 15, 2008 2:51 PM, John LaBanca <jlab...@google.com> wrote:
> That would be inconsistent with MenuBar.addItem(MenuItem), which returns
> void. However, I'm not personally against it if the benefit outweighs the
> inconsistency.

Oh. I've never actually used a MenuBar, so I didn't realize
addItem(MenuItem) worked that way. I would suggest changing
addItem(MenuItem) to return the passed-in MenuItem for the same
reasons. Changing a void method to return a MenuItem should be easy
since it's non-breaking, right?

Since I haven't used menus in GWT yet, perhaps you'd like someone
else's comments, too, but I find this construct:

Foo foo = obj.addFoo(new Foo());

more convenient than this:

Foo foo = new Foo();

obj.addFoo(foo);

The extra convenience, for me, is mostly in the reduced line count, so
it's not a big deal one way or the other.

[Bruce Johnson]

I'm for both of Ian's suggestions, unless anyone can think of a downside. His code example:

T x = foo(new T());

is pretty persuasive in terms of convenience. If it's something the compiler can't yet optimize well, it's worth making the compiler smarter to support this pattern.

My 2 cents.

-- Bruce 

[Emily Crutcher]

+1

--
"There are only 10 types of people in the world: Those who understand binary, and those who don't"

[walden]

-1

I'm not really against it, but I doubt I'd ever use it. UI parts that
I want to hang onto tend to be instance variables in my code, so their
declaration is usually separate from their assignment.

I think the proposed syntax makes sense when the call is designed for
chaining, but not otherwise, and I don't think this is a chaining use
case. A chaining example would be
StringBuffer.append(...).append(...)...toString();

My UI code will tend to look like this:

T x;

...

foo(x = new T());

If the real meaning of foo() is all about side-effects, then the above
code is *a little bit* more meaningful and readable to me. But no
biggie either way.


Cheers,
Walden

On Jan 15, 3:14 pm, "Emily Crutcher" <e...@google.com> wrote:
> +1
>
> On Jan 15, 2008 3:03 PM, Bruce Johnson <br...@google.com> wrote:
>
>
>
>
>
> > I'm for both of Ian's suggestions, unless anyone can think of a downside.
> > His code example:
> > T x = foo(new T());
>
> > is pretty persuasive in terms of convenience. If it's something the
> > compiler can't yet optimize well, it's worth making the compiler smarter to
> > support this pattern.
>
> > My 2 cents.
> > -- Bruce
>

> > On Jan 15, 2008 2:58 PM, Ian Petersen <ispet...@gmail.com> wrote:

> binary, and those who don't"- Hide quoted text -
>
> - Show quoted text -

[Robert Hanson]

I typically prefer void methods to return the instance (the instance
that owns the method) to allow for chaining.

menuBar
.addItem(new MenuItem("Foo", new FooCommand()))
.addSeparator()
.addItem(new MenuItem("Bar", new BarCommand()))
.addItem(new MenuItem("Baz", new BazCommand()));

In practice is there often a need to get a handle on the MenuItem
after it is created? Looking at the API for MenuItem I can't see that
there is much need for the object after it is added to the MenuBar.

This goes doubly for the separator... what method might you need to
call on the separator after it has been added to the MenuBar? I am
sure that there are going to be some edge cases, but I would expect
these to be few compared to the overall usage.

Rob

On Jan 15, 2008 3:03 PM, Bruce Johnson <br...@google.com> wrote:

[Isaac Truett]

While I can see the appeal of chaining the menuBar.add*() calls, I
don't really like the look of code it produces. I just see it being
auto-formatted into something like this:

menuBar.addItem(new MenuItem("Foo", new FooCommand())).addSeparator()

.addItem(new MenuItem("Bar", new BarCommand())).addItem(
new MenuItem("Baz", new BazCommand()));

I certainly will admit that his is a rather weak argument for an API
decision, but it seems to me that this is more a matter of consistency
and convenience than needed functionality.

The void return type doesn't bother me but if I had to choose another
return type, I would choose MenuItemSeparator over MenuBar. I could
see wanting a reference to the separator for modifying style names.

[Andrés Testi]

Chaining you are proposing would be replaced with a varargs method.
The single addItem would still return void, and code optimizations
would be done:

void addItems(MenuItem... items)

menuBar.addItems(
new MenuItem("Foo", new FooCommand()),
MenuItem.SEPARATOR,
new MenuItem("Bar", new BarCommand()),

new MenuItem("Baz", new BazCommand())
);

> > On Jan 15, 2008 2:58 PM, Ian Petersen <ispet...@gmail.com> wrote:

[Ray Cromwell]

On a side note, I wonder if there is a potential loss or gain for
future optimizers in terms of analysis difficulty and low hanging
fruit optimizations:

T x = new T()
foo(x)

vs

T x = foo(new T())

On first glance, x is known trivially known to be of type T in the
first, but in the second, x could be any subclass of T and not
neccessarily even the one that was passed in. In both cases, foo can
potentially do mutations or hang onto the reference.

-Ray

[Andrés Testi]

I think the first case you are exposing is more optimizable than the
second, because x is instantiated in the same scope where x is
defined. Then all the T method can be assumed as final in this scope,
and are all inlineable:

T x = new T();
foo(x)

x.doSomething(); // doSomething() would be assumed as final

-- Andrés

On 18 ene, 14:41, "Ray Cromwell" <cromwell...@gmail.com> wrote:
> On a side note, I wonder if there is a potential loss or gain for
> future optimizers in terms of analysis difficulty and low hanging
> fruit optimizations:
>
> T x = new T()
> foo(x)
>
> vs
>
> T x = foo(new T())
>
> On first glance, x is known trivially known to be of type T in the
> first, but in the second, x could be any subclass of T and not
> neccessarily even the one that was passed in. In both cases, foo can
> potentially do mutations or hang onto the reference.
>
> -Ray
>

[Miguel Ping]

I must've misinterpreted the optimization. Why can't I do:

T x = new Y(); //Y extends T :)

Back to the menuItem, I would prefer that the method returned void for
the sake of consistency. If chaining is to be supported, I prefer
that the other methods return the new item also (addMenuItem()) , not
only the addSeparator() method.

On Jan 18, 4:41 pm, "Ray Cromwell" <cromwell...@gmail.com> wrote:
> On a side note, I wonder if there is a potential loss or gain for
> future optimizers in terms of analysis difficulty and low hanging
> fruit optimizations:
>
> T x = new T()
> foo(x)
>
> vs
>
> T x = foo(new T())
>
> On first glance,  x is known trivially known to be of type T in the
> first, but in the second, x could be any subclass of T and not
> neccessarily even the one that was passed in. In both cases, foo can
> potentially do mutations or hang onto the reference.
>
> -Ray
>

> > > > > Ian- Hide quoted text -

[Ray Cromwell]

In that case the compiler can easily tighten the type of the lvalue
to Y. Not that it cant do it from method calls, only that it can
require more complicated inferences. I'm not sure gwt does these at
the moment.

[John LaBanca]

Returning the MenuItem that is passed into the function seems like the strongest argument, so the new methods look like:

public MenuItem addItem(MenuItem item)
public MenuItemSeparator addSeparator(MenuItemSeparator separator)

Returning the MenuBar is inconsistent with the current addItem methods that already return the new MenuItem.  See the other thread for the patch:
http://groups.google.com/group/Google-Web-Toolkit-Contributors/browse_thread/thread/67d9f19d53273e8f

>> On Jan 18, 2008 7:51 AM, Andr�s Testi <andres.a.te...@gmail.com > w