small code review: Add InlineHyperlink widget

Hello Ray :)

Would you take a look at this patch for the 1.6 branch, adding the
InlineHyperlink widget?

It adds a protected constructor to Hyperlink, allowing subclasses to
pass a wrapper element. If that element is null, it calls setElement
with the anchor element. InlineHyperlink then is just a Hyperlink
widget with no wrapping div.

Associated issue is here:
http://code.google.com/p/google-web-toolkit/issues/detail?id=3056

Thanks!

--
Alex Rudnick
swe, gwt, atl

[rjrjr, 2008-12-03 01:40:32]

LGTM with the nits below fixed.

http://codereview.appspot.com/8700/diff/1/3
File user/src/com/google/gwt/user/client/ui/Hyperlink.java (right):

http://codereview.appspot.com/8700/diff/1/3#newcode45
Line 45: * <h3>CSS Style Rules</h3> <ul class='css'> <li>.gwt-Hyperlink { }</li>
</ul>
Autoformat damage. Please don't autoformat entire existing classes.

http://codereview.appspot.com/8700/diff/1/3#newcode56
Line 56: private Element anchorElem;
can you make this final?

http://codereview.appspot.com/8700/diff/1/2
File user/src/com/google/gwt/user/client/ui/InlineHyperlink.java (right):

http://codereview.appspot.com/8700/diff/1/2#newcode25
Line 25: * <h3>CSS Style Rules</h3> <ul class='css'> <li>.gwt-InlineHyperlink {
}</li>
shouldn't be in one line

http://codereview.appspot.com/8700/diff/1/2#newcode64
Line 64: public InlineHyperlink(String text, String targetHistoryToken) {
I'd expect this to be
this(text, false, targetHistoryToken);

[ajr, 2008-12-03 16:54:05]

Thanks for taking a look!

Fixed the nits and committed r4241.

http://codereview.appspot.com/8700/diff/1/3
File user/src/com/google/gwt/user/client/ui/Hyperlink.java (right):

http://codereview.appspot.com/8700/diff/1/3#newcode45
Line 45: * <h3>CSS Style Rules</h3> <ul class='css'> <li>.gwt-Hyperlink { }</li>
</ul>
On 2008/12/03 01:40:33, rjrjr wrote:
> Autoformat damage. Please don't autoformat entire existing classes.

Fixed!

http://codereview.appspot.com/8700/diff/1/3#newcode56
Line 56: private Element anchorElem;
On 2008/12/03 01:40:33, rjrjr wrote:
> can you make this final?

Made final!

http://codereview.appspot.com/8700/diff/1/2
File user/src/com/google/gwt/user/client/ui/InlineHyperlink.java (right):

http://codereview.appspot.com/8700/diff/1/2#newcode25
Line 25: * <h3>CSS Style Rules</h3> <ul class='css'> <li>.gwt-InlineHyperlink {
}</li>
On 2008/12/03 01:40:33, rjrjr wrote:
> shouldn't be in one line

Fixed.

http://codereview.appspot.com/8700/diff/1/2#newcode64
Line 64: public InlineHyperlink(String text, String targetHistoryToken) {
On 2008/12/03 01:40:33, rjrjr wrote:
> I'd expect this to be
> this(text, false, targetHistoryToken);

Good point. Fixed