-
Notifications
You must be signed in to change notification settings - Fork 257
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
Permit configurable replacement of search tracking with client session storage #2954
base: main
Are you sure you want to change the base?
Permit configurable replacement of search tracking with client session storage #2954
Conversation
- allows client-side management of show-view result set navigation
… rather than boolean
…tion in show view
… in show views - use client SearchContext components when blacklight_config.track_search_session is 'client'
3f9cac9
to
c097179
Compare
app/components/blacklight/search_context/client_applied_params_component.html.erb
Outdated
Show resolved
Hide resolved
app/components/blacklight/search_context/client_applied_params_component.rb
Outdated
Show resolved
Hide resolved
}) | ||
}) | ||
} | ||
Blacklight.onLoad(function(){ |
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.
Is it necessary to use onLoad here or can this function be called by doSearchContextBehavior
? I think it's confusing to depend on both approaches.
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.
doSearchContextBehavior
adds an event listener for clicks, which doesn't inspect the DOM (yet). The "page links" behavior in the show view depends on DOM content. I'm trying to refactor.
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.
ok, pushed a refactor up. I think that I have to either piggyback on Blacklight.onLoad
's event detection, or I would have to reimplement it. I at least relocated the event handler attachment and the onload addition to doSearchContextBehavior
.
…search_session - allow configuration of components for applied parameters and itewm pagination
- use const over let wherever possible
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.
It's important to document the implications of this PR. Where?
I think it'd be good to split the search history tracking from search context pagination. Spotlight, for example, has a pretty significant dependency on search history, but I think it'd be great to get this better client-side context in play too. I don't think that's necessarily a blocker to merging this, but would be a nice compatibility thing for downstream users. |
I updated the PR description to be more explicit about the changes here. |
@cbeer I think it would be good to redesign saving a search as an explicit user action? Have a button on the index view if you are logged in to save the search, rather than passively logging searches. |
@barmintor This has a conflict and needs to be rebased (and re-build the javascript). |
fixes #2558
Implement separate client-side and server-side components for tracking search session, displaying appliedParams block in show view, and displaying result pagination from show view
Configuration
The
track_session_storage
configuration changes in this PR from a true/false flag to either false or a configuration object that responds to the keysstorage, applied_params_component, item_pagination_component
The default value for this configuration in this PR is:
This configuration reproduces the legacy enabled behavior; setting the configuration to
false
is also supported.The new configuration available is:
Index Behavior
In the
storage: "client"
configuration, clicking on a search result stores the current search (query string) insessionStorage
, and submits a form that GETs the show view with acounter
parameter (without POSTing to the track URL). The page links information (ie, previous/next and result set size/offset display) is added dynamically with data fetched from the current search controller in the show view. This dynamically generated form is not used if the item is being right-clicked or context-clicked (for example, when opening in a new tab).Show Behavior
In the
storage: "client"
configuration, a show view that has a URL param forcounter
will fetch JSON data from a new controller action calledpage_links
that returns previous/next information based on the counter and the currently stored search. If no search is available in the sessionStorage or no data is returned, the scaffolding for search context is removed.