-
Notifications
You must be signed in to change notification settings - Fork 95
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
ENH Keep State and show Search field if any keywords are presented #1326
ENH Keep State and show Search field if any keywords are presented #1326
Conversation
Open instead of #1322 |
92e3ac5
to
04a92be
Compare
e9c815c
to
e5980dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR should target the 1
branch - it doesn't make any sense to release this separately from silverstripe/silverstripe-framework#10331
The UX advice we received indicated the following behaviour for the history state:
- If you navigate to a
ModelAdmin
or page that has a gridfield and then immediately click 'back' in your browser, you should go back to the previous page you were on. - If you navigate to a
ModelAdmin
or page that has a gridfield and then change the state of that gridfield (either one time or many times e.g. sort, filter, paginate a couple of times), when you click 'back' in your browser you should go back to the initial default state of the gridfield. Pressing 'back' again will then take you back to the page you were previously on.
The first of those is met, but the second is not.
b4f5f6f
to
3aae27e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add behat tests for navigating back/forward in the browser.
This can be done with When I move backward one page
and When I move forward one page
Also, have you addressed my earlier comment about the UX advice not being implemented correctly?
#1326 (review) |
If you click the "back" button in the browser, and then click the "forward" button in the browser, you should be looking at the exact same page with the same state as you had before you clicked "back". |
Sorry, Guy, but I completely disagree with this approach. |
6ca493b
to
e72ed94
Compare
Just putting this here for the sake of anyone coming across this PR in the future:
i.e:
|
e72ed94
to
eb61b0d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One very minor change to make.
eb61b0d
to
55beb10
Compare
client/src/legacy/GridField.js
Outdated
const historyState = $.extend({}, { page: window.location.pathname + searchParam }, this.getState()); | ||
history.replaceState(historyState, '', window.location.pathname + searchParam); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const historyState = $.extend({}, { page: window.location.pathname + searchParam }, this.getState()); | |
history.replaceState(historyState, '', window.location.pathname + searchParam); | |
window.ss.router.replace(window.location.pathname + searchParam, undefined, undefined, false); |
Turns out we have to use the special custom page router that Silverstripe admin uses... see https://github.com/visionmedia/page.js/blob/master/page.js#L678 for what this is actually doing.
client/src/legacy/GridField.js
Outdated
|
||
$(window).on('popstate', function (e) { | ||
if (e.originalEvent.state === null) { return; } | ||
location.reload(); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what's causing the behat failures.
$(window).on('popstate', function (e) { | |
if (e.originalEvent.state === null) { return; } | |
location.reload(); | |
}); |
55beb10
to
b86259d
Compare
Description
Parent issue