-
Notifications
You must be signed in to change notification settings - Fork 312
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
Decouple Instant Results from the Modal #3159
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Check if facetSearchInput exists.
Add missing );
Support multiple Facets
Revert "Support multiple Facets"
Merge develop into master in prep for 3.5.5
Merge develop into master for 3.5.6 release
Add captions to WP.org screenshots
Update the trunk branch
Update trunk
ElasticPress 4.0 Beta 1 Notice
JakePT
changed the title
WIP: Decouple Instant Results from the Modal
Decouple Instant Results from the Modal
Jan 19, 2023
felipeelia
added
the
module:instant-results
Issues related to the Instant Results functionality
label
Jan 20, 2023
1 task
This was referenced Sep 5, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of the Change
This change decouples Instant Results from the modal UI. This is achieved by moving the state logic out of the modal to an
ApiSearchProvider
provider component and providing a custom hookuseApiSearch()
for consuming its state and methods. The various Instant Results components have been updated to use this hook instead of the generic Context.The provider component is not currently publicly exposed, but that's something that could be possible in a future version once its API is more stable.
The main challenge was how to handle the open/closed state of the modal, since this was coupled with the history API functionality so that back and forward could properly open and close the modal. The problem being that this open/close logic is related to the modal UI and not the rest of the state. The solution I came up with was to have a generic on/off state of the provider that the Modal UI could control and whose state it could reflect to open and close the modal, while other UI implementations could theoretically use it differently.
A side effect of some of the improvements made here is that a bug where an Instant Results request would be sent on every page load, even if the modal was never opened, has been fixed. Now an API request is only sent when the modal is opened.
Closes #2979
How to test the Change
Changelog Entry
Changed - Under the hood improvements to the structure of Instant Results.
Fixed - An issue where API requests for Instant Results would be sent on page load before the modal has been opened.
Credits
@JakePT
Checklist: