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

Modify actions in search page triggering a page reload #2798

Merged
merged 8 commits into from Sep 9, 2016

Conversation

Projects
None yet
2 participants
@edmundoa
Member

edmundoa commented Sep 8, 2016

Description

Many actions in the search page trigger a full page reload, either because they use links to navigate to another page, or because they submit a form to the server.

That was needed in previous versions of the web interface (prior 2.0) but now can be replaced by a combination of react-router and a refresh of the reflux stores involved in the search. This has the advantage of avoiding the page reload, avoiding asking the server for the web interface assets (as described in #2488), and also avoiding flashes with the login page (as described in #2770).

This PR takes care of modifying some of those actions to avoid page reloads when:

  • Executing searches
  • Using search pagination
  • Changing histogram resolution
  • Selecting saved searches
  • Changing message sorting

This PR intentionally does not take care of:

  • Refactoring the code around searches (specially SearchStore), as it is too risky for a bugfix release. This is something we can move into 2.2
  • Avoiding page reload when using surrounding searches. The main problem here is to reset the search view, e.g. scroll to top, reset page number, and collapse expanded message. Doing those things would require more changes that I don't think are worth pursuing right now. We can take care of fixing it in a follow-up task

Motivation and Context

The changes were motivated by #2488, and they also fix that issue.

How Has This Been Tested?

I have checked that these search elements still work after the changes:

  • Widgets use right search params
  • Save searches use right search params
  • CSV export uses right search params (but didn't solve the issue described in #2795)
  • Field analyzers use right params
  • Searching from sources tab works
  • Searching in streams works
  • Search live-loading works
  • Show surrounding messages works
  • Sorting works
  • Field selection works

edmundoa added some commits Sep 7, 2016

Avoid page reload when a search param changes
When changing the search page or histogram resolution, do not force a
full page reload, but change the URL. That triggers an update of the
stores involved in the search.

Refs #2488
Avoid page reload on search submit
Prevent the submit of new searches, and change the URL instead. That
triggers an update on the stores involved in the search page.

Refs #2488
Use latest search to get widget parameters
Delay fetching search parameters to create widgets until the last
moment, so we ensure the parameters used are the ones from the latest
search performed.
Avoid page reload when loading saved search
Instead replace URL and let search stores to fetch new data.

Refs #2488
Fix search page sorting
After the last changes in the search reload behaviour, remove the
`sortField` and `sortOrder` options from `SearchResult`'s state and rely
on `SearchStore` to provide the right values.

@edmundoa edmundoa added the web label Sep 8, 2016

@edmundoa edmundoa added this to the 2.1.1 milestone Sep 8, 2016

@dennisoelkers dennisoelkers self-assigned this Sep 8, 2016

Add search loading indicator
Since we don't reload the search page now, we need a way of giving
feedback to the user when the search is being loaded. This commit
introduces a loading indicator in the bottom right corner of the screen
for such purpose.
let loadingIndicator;
if (this.props.loadingSearch) {
loadingIndicator = <div className="loading-indicator"><Spinner text="Updating search results..."/></div>;

This comment has been minimized.

@dennisoelkers

dennisoelkers Sep 9, 2016

Member

I think it makes sense to extract this into a separate component straight away, that we can reuse in different places too, including moving its styling from the global stylesheet to a local one.

This comment has been minimized.

@edmundoa

edmundoa Sep 9, 2016

Member

Makes sense, will take care of it

@@ -332,14 +333,18 @@ class SearchStore {
var searchURLParams = this.getOriginalSearchURLParams();
searchURLParams = searchURLParams.set("width", this.width);
searchURLParams = searchURLParams.set(param, value);
URLUtils.openLink(this.searchBaseLocation("index") + "?" + Qs.stringify(searchURLParams.toJS()), false);

This comment has been minimized.

@dennisoelkers

dennisoelkers Sep 9, 2016

Member

As URLUtils.openLink is not used anymore, can you please remove it?

This comment has been minimized.

@edmundoa
@dennisoelkers

This comment has been minimized.

Member

dennisoelkers commented Sep 9, 2016

Besides those two remarks, it looks good to me.

@dennisoelkers dennisoelkers merged commit f51e5ca into 2.1 Sep 9, 2016

4 checks passed

ci-server-integration Jenkins build graylog2-server-integration-pr 1349 has succeeded
Details
ci-web-linter Jenkins build graylog-pr-linter-check 832 has succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@dennisoelkers dennisoelkers deleted the issue-2488 branch Sep 9, 2016

dennisoelkers added a commit that referenced this pull request Sep 9, 2016

Modify actions in search page triggering a page reload (#2798)
* Avoid page reload when a search param changes

When changing the search page or histogram resolution, do not force a
full page reload, but change the URL. That triggers an update of the
stores involved in the search.

Refs #2488

* Avoid page reload on search submit

Prevent the submit of new searches, and change the URL instead. That
triggers an update on the stores involved in the search page.

Refs #2488

* Use latest search to get widget parameters

Delay fetching search parameters to create widgets until the last
moment, so we ensure the parameters used are the ones from the latest
search performed.

* Avoid page reload when loading saved search

Instead replace URL and let search stores to fetch new data.

Refs #2488

* Fix search page sorting

After the last changes in the search reload behaviour, remove the
`sortField` and `sortOrder` options from `SearchResult`'s state and rely
on `SearchStore` to provide the right values.

* Add search loading indicator

Since we don't reload the search page now, we need a way of giving
feedback to the user when the search is being loaded. This commit
introduces a loading indicator in the bottom right corner of the screen
for such purpose.

* Remove unused openLink method

* Make search loading indicator reusable

(cherry picked from commit f51e5ca)

@bernd bernd referenced this pull request Sep 12, 2016

Closed

app reloads on search #2488

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment