Change in gwt[master]: Add History.getUrl(String token) which returns the correspon...

[Johannes Barop]

Johannes Barop has uploaded a new change for review.

https://gwt-review.googlesource.com/2960


Change subject: Add History.getUrl(String token) which returns the
corresponding URL for a history token.
......................................................................

Add History.getUrl(String token) which returns the corresponding URL for a
history token.

This enables custom History implementations (for example a implementation
with pushstate) to modify the display of URLs in widgets (for example
Hyperlink).

Change-Id: I9f9b261547b539489d9cee8a72f660264d64e9f2
---
M user/src/com/google/gwt/user/client/History.java
M user/src/com/google/gwt/user/client/impl/HistoryImpl.java
M user/src/com/google/gwt/user/client/ui/Hyperlink.java
3 files changed, 20 insertions(+), 2 deletions(-)



diff --git a/user/src/com/google/gwt/user/client/History.java
b/user/src/com/google/gwt/user/client/History.java
index 285ffee..03a29e8 100644
--- a/user/src/com/google/gwt/user/client/History.java
+++ b/user/src/com/google/gwt/user/client/History.java
@@ -18,6 +18,7 @@
import com.google.gwt.core.client.GWT;
import com.google.gwt.core.client.impl.Disposable;
import com.google.gwt.core.client.impl.Impl;
+import com.google.gwt.dom.client.AnchorElement;
import com.google.gwt.event.logical.shared.ValueChangeEvent;
import com.google.gwt.event.logical.shared.ValueChangeHandler;
import com.google.gwt.event.shared.HandlerManager;
@@ -240,4 +241,14 @@
WrapHistory.remove(impl.getHandlers(), listener);
}
}
+
+ /**
+ * Gets the corresponding URL for the given history token.
+ *
+ * @param historyToken History token to generate the URL for.
+ */
+ public static String getUrl(String historyToken) {
+ return impl.getUrl(historyToken);
+ }
+
}
diff --git a/user/src/com/google/gwt/user/client/impl/HistoryImpl.java
b/user/src/com/google/gwt/user/client/impl/HistoryImpl.java
index af675cd..d960111 100644
--- a/user/src/com/google/gwt/user/client/impl/HistoryImpl.java
+++ b/user/src/com/google/gwt/user/client/impl/HistoryImpl.java
@@ -16,12 +16,16 @@
package com.google.gwt.user.client.impl;

import com.google.gwt.core.client.JavaScriptObject;
+import com.google.gwt.dom.client.AnchorElement;
import com.google.gwt.event.logical.shared.HasValueChangeHandlers;
import com.google.gwt.event.logical.shared.ValueChangeEvent;
import com.google.gwt.event.logical.shared.ValueChangeHandler;
import com.google.gwt.event.shared.GwtEvent;
import com.google.gwt.event.shared.HandlerManager;
import com.google.gwt.event.shared.HandlerRegistration;
+import com.google.gwt.user.client.DOM;
+import com.google.gwt.user.client.Element;
+import com.google.gwt.user.client.History;

/**
* Native implementation associated with
@@ -149,6 +153,10 @@
}
}

+ public String getUrl(String historyToken) {
+ return "#" + History.encodeHistoryToken(historyToken);
+ }
+
protected native String decodeFragment(String encodedFragment) /*-{
// decodeURI() does *not* decode the '#' character.
return decodeURI(encodedFragment.replace("%23", "#"));
diff --git a/user/src/com/google/gwt/user/client/ui/Hyperlink.java
b/user/src/com/google/gwt/user/client/ui/Hyperlink.java
index 61b9821..3366aa4 100644
--- a/user/src/com/google/gwt/user/client/ui/Hyperlink.java
+++ b/user/src/com/google/gwt/user/client/ui/Hyperlink.java
@@ -342,8 +342,7 @@
assert targetHistoryToken != null
: "targetHistoryToken must not be null, consider using Anchor
instead";
this.targetHistoryToken = targetHistoryToken;
- String hash = History.encodeHistoryToken(targetHistoryToken);
- DOM.setElementProperty(anchorElem, "href", "#" + hash);
+ DOM.setElementProperty(anchorElem, "href",
History.getUrl(targetHistoryToken));
}

public void setText(String text) {

--
To view, visit https://gwt-review.googlesource.com/2960
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I9f9b261547b539489d9cee8a72f660264d64e9f2
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Johannes Barop <j...@barop.de>

[Johannes Barop]

Johannes Barop has posted comments on this change.

Change subject: Add History.getUrl(String token) which returns the
corresponding URL for a history token.
......................................................................

Patch Set 1:

Please note the missing test for History.getUrl as I unsure how to write a
good test case for it. I'm open for suggestions

--
To view, visit https://gwt-review.googlesource.com/2960
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: I9f9b261547b539489d9cee8a72f660264d64e9f2
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Johannes Barop <j...@barop.de>

Gerrit-Reviewer: Johannes Barop <j...@barop.de>
Gerrit-HasComments: No

[Johannes Barop]

Johannes Barop has uploaded a new patch set (#2).

Change subject: Add History.getUrl(String token) which returns the
corresponding URL for a history token.
......................................................................

Add History.getUrl(String token) which returns the corresponding URL for a
history token.

This enables custom History implementations (for example a implementation
with pushstate) to modify the display of URLs in widgets (for example
Hyperlink).

Change-Id: I9f9b261547b539489d9cee8a72f660264d64e9f2
---
M user/src/com/google/gwt/user/client/History.java
M user/src/com/google/gwt/user/client/impl/HistoryImpl.java
M user/src/com/google/gwt/user/client/ui/Hyperlink.java

3 files changed, 17 insertions(+), 2 deletions(-)

--
To view, visit https://gwt-review.googlesource.com/2960
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f9b261547b539489d9cee8a72f660264d64e9f2
Gerrit-PatchSet: 2

[Jens Nehlmeier]

Jens Nehlmeier has posted comments on this change.

Change subject: Add History.getUrl(String token) which returns the
corresponding URL for a history token.
......................................................................

Patch Set 2:

(2 comments)

....................................................
File user/src/com/google/gwt/user/client/History.java
Line 247: *
Nit-pick: Trailing whitespace at end of line.


Line 250: public static String getUrl(String historyToken) {
getRelativeUrl() is probably a bit more descriptive?

--
To view, visit https://gwt-review.googlesource.com/2960
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9b261547b539489d9cee8a72f660264d64e9f2
Gerrit-PatchSet: 2

Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Johannes Barop <j...@barop.de>

Gerrit-Reviewer: Jens Nehlmeier <jens.ne...@gmail.com>
Gerrit-HasComments: Yes

[Goktug Gokdogan]

Goktug Gokdogan has posted comments on this change.

Change subject: Add History.getUrl(String token) which returns the
corresponding URL for a history token.
......................................................................

Patch Set 2:

I'm still not sure how we want to move forward with pushstate. So I'm
hesitant to add a new public API to History. Can you look for an
alternative where we don't need to expose a new public API?

--
To view, visit https://gwt-review.googlesource.com/2960
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9b261547b539489d9cee8a72f660264d64e9f2
Gerrit-PatchSet: 2

Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Johannes Barop <j...@barop.de>

Gerrit-Reviewer: Goktug Gokdogan <gok...@google.com>
Gerrit-Reviewer: Jens Nehlmeier <jens.ne...@gmail.com>
Gerrit-HasComments: No

[Johannes Barop]

Johannes Barop has posted comments on this change.

Change subject: Add History.getUrl(String token) which returns the
corresponding URL for a history token.
......................................................................

Patch Set 2:

The first alternative is HyperlinkImpl. But this would introduce spaghetti
inheritance because of the existing HyperlinkImpl's. And it clutters the
History stuff across multiple classes.

Other alternative: Add it only to HistoryImpl and let Hyperlink use
HistoryImpl directly. But this is not optimal because then we have to
duplicate the HistoryImpl.init() error handling and it feels hacky to me.

I think a new method to generate a link for a given history token is the
best way in the end because generating URLs is a part of implementing
History mechanisms.

This change makes the code of gwt-pushstate simpler. However I have already
a workaround for it. So it's not critical and we can take time to think
over it.

--
To view, visit https://gwt-review.googlesource.com/2960
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f9b261547b539489d9cee8a72f660264d64e9f2
Gerrit-PatchSet: 2

Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Johannes Barop <j...@barop.de>

Gerrit-Reviewer: Goktug Gokdogan <gok...@google.com>
Gerrit-Reviewer: Jens Nehlmeier <jens.ne...@gmail.com>

[Johannes Barop]

Johannes Barop has uploaded a new patch set (#3).

Change subject: Add History.getUrl(String token) which returns the
corresponding URL for a history token.
......................................................................

Add History.getUrl(String token) which returns the corresponding URL for a
history token.

This enables custom History implementations (for example a implementation
with pushstate) to modify the display of URLs in widgets (for example
Hyperlink).

Change-Id: I9f9b261547b539489d9cee8a72f660264d64e9f2
---
M user/src/com/google/gwt/user/client/History.java
M user/src/com/google/gwt/user/client/impl/HistoryImpl.java
M user/src/com/google/gwt/user/client/ui/Hyperlink.java

3 files changed, 16 insertions(+), 2 deletions(-)

--
To view, visit https://gwt-review.googlesource.com/2960
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f9b261547b539489d9cee8a72f660264d64e9f2
Gerrit-PatchSet: 3

Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Johannes Barop <j...@barop.de>