AppRouting should preserve hash on same-RouteId navigations #4219
Conversation
| // hash storage, set by tf-storage on the Polymer side. Due to async | ||
| // operators, we cannot guarantee that the initial 'navigated' action | ||
| // will be processed before any other URL-modifying logic. |
There was a problem hiding this comment.
I don't think it is purely about async operators.
For instance, if you are navigated to timeseries (#timeseries is written by HashStorage) and if you interact with the pinning, this effect will overwrite and remove #timeseries. In this case, HashStorage was not even attempt to write any hash so there is nothing timing related about this.
Could you more accurately depict the problem please?
Also I'd appreciate hash storage -> HashStorageComponent
There was a problem hiding this comment.
Updated the comment and dropped the part about async.
| }); | ||
|
|
||
| it('does not preserve hash upon replace for non-initial navigation', () => { | ||
| it('preserves hash upon navigations to the same route id', () => { |
There was a problem hiding this comment.
in this behavioral spec, maybe specify that this is a temporary behavior until all apps use the router?
stephanwlee
left a comment
stephanwlee
left a comment
There was a problem hiding this comment.
Thanks for updating the comments.
3 participants
Followup: #4220
AppRoutingEffects recently added logic to preserve URL hash
upon the initial 'navigated' action (see [1]). While this fixed cases
where the URL needs to be read upon initial page load, it does
not address an upcoming case needed for deeplinking pins in
Time Series.
Flow: pin a card, DeepLinkProvider serializes
?pinnedCards=in the URL which drops
#timeseries, reload the tab, theselected dashboard is no longer Time Series.
Expected: Time Series remains selected upon tab reload.
This change overcomes this issue the DeepLinkProvider by
preserving the hash upon same-RouteId navigations.
Manually checked internal TB embedders that navigating to
a different route id still clears the hash.
Googlers, see sync cl/335688801
[1] #4213