Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(react): expand the stack to better support back button, fixes #19748 #19856

Merged
merged 1 commit into from Nov 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 24 additions & 9 deletions packages/react-router/src/ReactRouter/Router.tsx
Expand Up @@ -22,7 +22,7 @@ class RouteManager extends React.Component<RouteComponentProps, RouteManagerStat
listenUnregisterCallback: UnregisterCallback | undefined;
activeIonPageId?: string;
currentDirection?: RouterDirection;
locationHistory: LocationHistory = new LocationHistory();
locationHistory = new LocationHistory();

constructor(props: RouteComponentProps) {
super(props);
Expand Down Expand Up @@ -77,7 +77,13 @@ class RouteManager extends React.Component<RouteComponentProps, RouteManagerStat
historyChange(location: HistoryLocation, action: HistoryAction) {
location.state = location.state || { direction: this.currentDirection };
this.currentDirection = undefined;
this.locationHistory.add(location);
if (action === 'PUSH') {
this.locationHistory.add(location);
} else if ((action === 'REPLACE' && location.state.direction === 'back') || action === 'POP') {
this.locationHistory.pop();
} else {
this.locationHistory.replace(location);
}
this.setState({
location,
action
Expand Down Expand Up @@ -111,21 +117,22 @@ class RouteManager extends React.Component<RouteComponentProps, RouteManagerStat
* If the page is being pushed into the stack by another view,
* record the view that originally directed to the new view for back button purposes.
*/
enteringView.prevId = enteringView.prevId || leavingView.id;
enteringView.prevId = leavingView.id;
} else if (action === 'POP') {
direction = leavingView.prevId === enteringView.id ? 'back' : 'none';
} else {
direction = direction || 'back';
leavingView.mount = false;
}
} else if (direction === 'back' || action === 'REPLACE') {
}
if (direction === 'back' || action === 'REPLACE') {
leavingView.mount = false;
this.removeOrphanedViews(enteringView, enteringViewStack);
}
} else {
// If there is not a leavingView, then we shouldn't provide a direction
direction = undefined;
}
this.removeOrphanedViews(enteringView, enteringViewStack);
} else {
enteringView.show = true;
enteringView.mount = true;
Expand All @@ -151,12 +158,13 @@ class RouteManager extends React.Component<RouteComponentProps, RouteManagerStat
if (enteringEl) {
// Don't animate from an empty view
const navDirection = leavingEl && leavingEl.innerHTML === '' ? undefined : direction === 'none' ? undefined : direction;
const shouldGoBack = !!enteringView.prevId || !!this.locationHistory.previous();
this.commitView(
enteringEl!,
leavingEl!,
viewStack.routerOutlet,
navDirection,
!!enteringView.prevId);
shouldGoBack);
} else if (leavingEl) {
leavingEl.classList.add('ion-page-hidden');
leavingEl.setAttribute('aria-hidden', 'true');
Expand All @@ -166,6 +174,8 @@ class RouteManager extends React.Component<RouteComponentProps, RouteManagerStat
}

removeOrphanedViews(view: ViewItem, viewStack: ViewStack) {
// Note: This technique is a bit wonky for views that reference each other and get into a circular loop.
// It can still remove a view that probably shouldn't be.
const viewsToRemove = viewStack.views.filter(v => v.prevId === view.id);
viewsToRemove.forEach(v => {
// Don't remove if view is currently active
Expand Down Expand Up @@ -339,15 +349,20 @@ class RouteManager extends React.Component<RouteComponentProps, RouteManagerStat
if (activeIonPage) {
const { view: enteringView } = this.state.viewStacks.findViewInfoById(activeIonPage.prevId);
if (enteringView) {
const lastLocation = this.locationHistory.findLastLocation(enteringView.routeData.match!.url);
const lastLocation = this.locationHistory.findLastLocationByUrl(enteringView.routeData.match!.url);
if (lastLocation) {
this.handleNavigate('replace', lastLocation.pathname + lastLocation.search, 'back');
} else {
this.handleNavigate('replace', enteringView.routeData.match!.url, 'back');
}
} else {
if (defaultHref) {
this.handleNavigate('replace', defaultHref, 'back');
const currentLocation = this.locationHistory.previous();
if (currentLocation) {
this.handleNavigate('replace', currentLocation.pathname + currentLocation.search, 'back');
} else {
if (defaultHref) {
this.handleNavigate('replace', defaultHref, 'back');
}
}
}
} else {
Expand Down
29 changes: 25 additions & 4 deletions packages/react-router/src/utils/LocationHistory.ts
Expand Up @@ -12,9 +12,30 @@ export class LocationHistory {
}
}

findLastLocation(url: string) {
const reversedLocations = [...this.locationHistory].reverse();
const last = reversedLocations.find(x => x.pathname.toLowerCase() === url.toLowerCase());
return last;
pop() {
this.locationHistory.pop();
}

replace(location: HistoryLocation) {
this.locationHistory.pop();
this.locationHistory.push(location);
}

findLastLocationByUrl(url: string) {
for (let i = this.locationHistory.length - 1; i >= 0; i--) {
const location = this.locationHistory[i];
if (location.pathname.toLocaleLowerCase() === url.toLocaleLowerCase()) {
return location;
}
}
return undefined;
}

previous() {
return this.locationHistory[this.locationHistory.length - 2];
}

current() {
return this.locationHistory[this.locationHistory.length - 1];
}
}