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

Version 1.5.5 #1117

Merged
merged 3 commits into from
Oct 1, 2020
Merged

Version 1.5.5 #1117

merged 3 commits into from
Oct 1, 2020

Conversation

oshi97
Copy link
Contributor

@oshi97 oshi97 commented Oct 1, 2020

Fixes:

oshi97 and others added 3 commits September 3, 2020 12:46
The event constructor does not work in ie11, but we have an
ie11 compliant implementation with DOM.trigger()

T=https://yextops.zendesk.com/agent/tickets/348126
TEST=manual

Test that before, in browserstack ie11,
 clicking the X button in a searchable facet has console error

With code change, no console error

and that it still clears the facet search input on chrome + ie11
Previously, there were two pagination persistence bugs.

Land on a vertical page with a query with results. Go to page 2. Go
to page 3. Try to navigate backwards in browser history. You will go
to page 1 instead of page 3.

Land on a vertical page w a query w results. Go to page 2. Refresh
the page. You will be on page 1 instead of page 2.

Both of these related bugs were because we were defaulting to resetting
pagination on any searchbar search. In 1, backwards navigation updates the
query and the searchbar takes the updated query and searches. In 2,
landing with a query query parameter is determined in the SearchBar
component onCreate.

To fix this, we add another value for the pre-existing queryTrigger
global storage key, QUERY_PARAMETER, meaning the query was triggered by
a query parameter.

Note: as a result of this fix, we were no longer able to go from Page 2
to Page 1. This is because of another bug, where navigating backwards in
history from presence of a query param to absence of the param will not
erase the parameter in storage. This is fixed for the pagination param here
but should be addressed more widely in SLAP-616.

J=SLAP-610
TEST=manual

Test on a local Jambo HH Theme site attached to a local dist of the SDK.
Test on a local site using the SDK directly.

Test that navigating on a vertical to Page 2 and then Page 3 allows you
to navigate back in browser history to Page 2 and Page 1 respectively.

Test that navigating to a page and then refreshing maintains pagination
offset. (IE Test that query params for search-offset work as intended).

Test that navigating to a page and then searching another query in the
searchbar /will/ reset pagination (and remove the associated query
parameter).

Test that queryTrigger in the liveapi request in all the situations
are null.

Test that with defaultInitialSearch set, queryTrigger still sends as
'initialize' in the liveapi request.
This commit downgrades regenerator-runtime from
^0.13.3 (currently 0.13.7) to being pinned to exactly
0.13.1. This is because there is a change in 0.13.2 which
adds back in a Function() constructor to define
regenerator runtime globally.
See #369 https://github.com/facebook/regenerator/commits/master/packages/regenerator-runtime

T=https://yextops.zendesk.com/agent/tickets/347915
TEST=manual

tested with a local apache httpd server on a searchbar only page
added below line to .htaccess, which sets the CSP to
the same as Syncreon

also test csp with a `<meta>` tag

tested both answers.js, answers.min.js, answers-modern.js, and answers-modern.min.js

double checked that our code still runs correctly in ie11, for both a searchbar only
page with no unsafe-eval csp, and a regular page with facets and results
that allows unsafe eval
Copy link
Contributor

@creotutar creotutar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎃 🎃 🎃 LGTM

@oshi97 oshi97 merged commit 8a084cf into master Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants