*1) The ButtonCell(DefaultAppearance.Resources) constructor can be
confusing, I think it should be dropped.
*This constructor gives the impression that you are using the app-wide
Appearance where in fact you are using DefaultAppearance. This could lead to
the following confusing use case:
- The user has two type of buttons: some initialized with new ButtonCell()
and some with new ButtonCell(otherCellButtonResources)
- He changes the style app-wide via <replace-with>
- Buttons initialized with new ButtonCell() change, the ones initialized
with new ButtonCell(otherCellButtonResources) don't.
Dropping that constructor would make it explicit you are actually forcing
the use of DefaultAppearance.
I would prefer the option. Lets say you have a 2 styles of buttons. A blue
cancel button and a grey standard type button. With the current proposal, I
would be able to use button and just inject different styles into the class.
With your proposal, I have to extend button to be able to provide a
different style than the norm for my cancel button, and what is worse, is
that the style substitution is now buried in the .gwt.xml file. I'd much
rather do my programming in the code than in .gwt.xml files. I would
definitely like the ability to override styles without being forced to
extend the widget classes.
*2) Providing a custom DefaultAppearance.Resources lead to somewhat smelly
code.
*DefaultAppearance.Resources ties buttonCellStyle() to a specific CSS
@Source. Therefore, I believe overriding it means writing something like:
public interface MyResources extends
ButtonCell.DefaultAppearance.Resources {
@Override
@Source("com/mygroup/myapp/client/ButtonCell.css")
Style buttonCellStyle();
}
Maybe it's just me, but this looks slightly smelly. Also, does GWT.create()
like two @Source for the same method?
Annotations on methods aren't inherited.
You can assign multiple css files in one @Source, so you could have
something like this.
public interface MyResources extends ButtonCell.DefaultAppearance.Resources
{
@Override
@Source({"com.google.gwt.user.client.ui.ButtonWidgetStyle.css",
"com/mygroup/myapp/client/ButtonCell.css"})
Style buttonCellStyle();
}
This would also allow you to make minor modifications to the default styling
without losing the ability to get updated styles for your widgets.
Unfortunately the* following is a compile error*
public interface MyResources extends ButtonCell.DefaultAppearance.Resources
{
String[] SOURCE =
{"com.google.gwt.user.client.ui.ButtonWidgetStyle.css"}
@Source(SOURCE)
Style buttonCellStyle();
}
otherwise I'd recommend making the DEFAULT_SOURCE a String[] so we could
easily add styles to it.
*3) Minor one-time style modifications are hard to make.
*Defining a custom Appearance or Resources is good when you want to style
all the buttons in your app, but impractical for small changes like
adjusting padding for a single button. We should think about a way to allow
that (and make sure it works in UiBinder). Ideas:
- Provide an addStyleName() in CellButton and Appearance. I don't like this,
however, as it creates the problem mentioned by Stephen (see point 4 below)
-- moreover, I really like the idea of Appearance moving us to more
"semantic" styling.
- Provide a setProperty(String name, String value) in CellButton and
Appearance and let Appearance apply it however it likes
- Provide semantic methods for frequently applied properties, like
setMargin(), setPadding()...
I'm really not sure I like any of the above proposals however. Maybe we
should just acknowledge that as a limitation of the new styling mechanism?
Is it too limiting?
Is that really necessary as ButtonWidget extends CellWidget which extends
Widget which gives you access to addStyleName and removeStyle name, making
it easy to add padding and margin around widgets without having to get all
Appearance.Resouce extendy.
*5) We should make sure GIN can be used to inject Appearance
into ButtonCell and Resources into Appearance.
*Weak coupling between ButtonCell, Appearance, and Resource seems like a
perfect use-case for DI. I have not reviewed the code from that perspective,
but it would be nice if we allowed users to subclass ButtonCell and
Appearance to provide @Inject-ed versions.
Agreed. Could/should we add javax.inject to the list of dependencies for GWT
allowing for DI on some of the components out of the box? Adding
javax.inject wouldn't force anyone to use Gin, but it definitely could
provide some api richness to those of us who do use it. I'll admit to not
having completely thought this through, but my initial reaction thought is
that this is a good idea.
*6) I agree with Thomas about:
* - Not being sure making Appearance an abstract class is the right choice.
I agree, I liked the idea of the Interface that has some method/final
constant like _extend_DefaultAppearnce_instead_implement_this
interface_at_your_own_peril.
That way new methods can be added and people who extended the interfaces...
they knew the risks and now they're getting burned.
- Favoring :hover and other pseudoclasses in the DefaultAppearance.
I also agree with this.
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
