Version 1.23.0 #933
Merged
Version 1.23.0 #933
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
* Changed most @media to @include breakpoints instead. Also moved answers-variable imports into common/variables. Changed $screen-xs-max value to $breakpoint-mobile-max and $screen-sm-min value to $breakpoint-mobile-min. J=SLAP-1277 TEST=manual Built the test site and checked on Chrome and IE11 for any unusual activity with varying window sizes. * changed back to pixels * Changed the utility mixin to use breakpoint-mobile-max and breakpoint-mobile-min instead of screen-xs-max and screen-sm-min * fixed pixel value of screen-xs-max and screen-sm-min
Merge master (Theme v1.22) into develop
Add unit tests to the runtime config model J=SLAP-1371 TEST=auto
An acceptance test for testing runtime config behavior This test confirms that runtime config set in a parent site is passed to the child runtime config (inside the iframe) J=SLAP-1371 TEST=acceptance
Add tests for AnswersExperienceFrame I created a jest mock for iframe-common so that we could check the interaction with the sendToIframe function J=SLAP-1371 TEST=auto
Unit tests for the manual initializer J=SLAP-1371 TEST=auto
Speed up the test site build step by moving some work from the build step to the setup step The setup-test-site command now does all the work to generate the jambo test site, and the build command does a regular `npx jambo build && grunt webpack` By moving the card and the vertical generation work to the setup step, the build step improves from 21.9 seconds to 7.4 seconds on my laptop. The downside is that some changes will now require a full setup to be ran where before it was possible to apply the change with the build command: - Updating page templates - Updating patch files - Updating the page config files J=none TEST=manual Test the speed improvements and smoke test the test site
Debounce the locator based on time This PR adds a time-based debouncer so that `searchThisArea` will not execute until 250ms have passed since the last time the debounce function was called. This prevents unnecessary searches from being ran when someone quickly zooms in or out of the map. For this change, I renamed the `SearchDebouncer` to the `SearchPreventer`since it is a more accurate name. The distinction is that if a search doesn't pass the criteria of the `SearchPreventer`, it will never be ran. Debouncing is different because when a debounced function is called, it will eventually execute after sufficient time has passed without another function call. I was hoping that this change would also reduce the number of searches made when dragging the map, however the debounce time necessary to make that work is too large and would lead to a sluggish user experience. It is possible that we would be able to accomplish it in a non-sluggish manner by reseting the debouncer whenever the user clicks on the map canvas, however further experimentation would be necessary. I opted not to debounce `searchThisArea()` calls after a user clicks on the 'Search This Area` button or on a cluster since there is little need for a debouncer there. J=SLAP-1301 TEST=manual Test that zooming in the map quickly doesn't trigger a new search until the zooming is done
Improve development build time and web performance by adding a button legacy build Create a legacy bundle build similar to #828 so that we can avoid the ponyfills and polyfills for modern web browsers. This yeids the following benefits: - Improved webpack build time by about 13% from 4.5 seconds to 4 seconds when IS_DEVELOPMENT_PREVIEW=true - Modern browsers are served a much smaller overlay-button bundle (32KB rather than the 211KB of the legacy bundle) Since specifying type="module" on the modern bundle makes it deferred, we must wait for DOMContentLoaded before trying to access `window.OverlayButtonJS` J=SLAP-1417 TEST=manual Smoke test the overlay on chrome and on IE11 on browserstack.
Theme's runtime config setters for analyticsEventsEnabled update SDK's config accordingly after Answers is initialized - update runtimeConfig to support general and key-specific listener with event type - changed AnswersExperience to a class - provide runtimeConfig a key-specific listener to handle updates on 'analyticsEventsEnabled' from AnswersExperience - a deferred promise is placed in the window (from html.hbs), resolve after Answers is initialized and call SDK's analytics setter, reject if Answers failed to initialize. - config updates from AnswersExperienceFrame send message to iframe and handle by RuntimeConfigReceiver. Since this invokes runtimeConfig.set from AnswersExperience, the same key-specific listener will be used. J=SLAP-1393 TEST=manual & auto Launched hitchhiker theme test site, called runtimeConfig.set for analyticsEventsEnabled in iframe and non-iframe page. Set analyticsEventsEnabled to false/true, and see that console.log from SDK's analytics setter printed the correct result. Click thumbs up on direct answer card, and confirm a network call is made to realtimeanalytics when set to true and no feedback got send when set to false. Update and test general/key-specific listener in jest Co-authored-by: Yen Truong <ytruong@yext.com>
Adds a translation test github action, that creates a messages.pot using jambo extract-translations and then checks that every .po file matches messages.pot. This is the same logic that the SDK's translation test uses. Also adds an Italian translation that was missing, so that tests pass. Previously, the translation for `vertical-full-page-map-alternative-verticals/alternativeverticals.hbs:47` was used for `vertical-full-page-map-alternative-verticals/alternativeverticals.hbs:5` instead of the actual line 5 translation, causing the translation for line 47 to be doubled and the translation for line 5 to be missing. J=SLAP-1378 TEST=auto checked that the test will run on master and release branches
Add support for 'sessionTrackingEnabled' in the runtime config This PR moves the canonicalization of the runtime config listeners into RuntimeConfig itself. The waiting for Answers to initialize is also moved out of the listeners and into `AnswersExperience`. Note: Currently when sessionTrackingEnabled is set, the ordering of the verticals on the universal results page changes. I suspect this is an issue with the answers-search-ui and therefore the issue is not related to this PR. J=SLAP-1410 TEST=manual Test both setting both 'analyticsEventsEnabled' and 'sessionTrackingEnabled' from an iframe integration and a standard integration
This lets us simplify our grunt watch a little bit. We also get some other benefits, like the stdout and stderr being logged in the original order jambo output them, as opposed to all of stderr and then all of stdout. This change also adds support for colorful jambo, at least when grunt watch is used via CLI. Previously, if you tried to run grunt watch, and hooked it up to a jambo with colored output, the colors wouldn't print. While working on this were some issues with getting the webpack build to not run if the jambo build failed. I tried switching to grunt-shell, but after some testing noticed that grunt-shell does not support colorful output, at least not with jambo nor with npm. So I just made an npm build script that would jambo build and then webpack. When testing this in live preview, I noticed that I needed to run `npm run build` with the --silent flag in order to suppress extra error messaging from npm, with the error messaging in question really messing up the sitesgitstorm console. These error messages occur when using node 12, but not on node 14 (what I used locally). I also moved the imports for the jambo config and webpack config into the Gruntfile's exported function. I think this will let people upgrade their themes without having to restart preview, at the very least it let me change the webpack config and gruntfile without needing to restart. J=SLAP-964 TEST=manual tested this change in sitesgitstorm on our slapshot test account. tried both changing a config file to trigger a build, and deleting a comma in a config file to trigger a build error tested that the webpack build will not try to run if the jambo build fails tested that colorful output works locally by hooking up my local jambo in SGS, tested manually editing the webpack config (not via upgrade) and saw my new console.log changes reflected in the console also updated the Gruntfile to not run npm run build with the --silent flag, intentionally broke a config json's syntax, and saw the oddly formatted npm error messages come back
This is the work needed before replacing IconComponent usages with iconPartial. Since there are many icon usages, it wouild be hard to see these changes if they were combined with the replacements. - register the relativePathHandler for SDK components so it can be used at runtime - fix bug in relativePathHandler where if the url is undefined, it will return something like "./undefined" instead of the value undefined. In previous usages of the helper, url was guarded from being null/undefined, but this won't be the case for iconPartial - add percy snapshots for a card with custom cta icons - fix test-sites gitignore negations not working properly - point the theme's test-site to the feature/icon-partial-i18n so we get the icon-partial change, and also i18n-ed assets so the spanish snapshots work - also collect code coverage on the hbshelpers folder J=SLAP-1297 TEST=manual,auto test that custom icons still work, both for section titles on universal and inside cards unit tests for relativePathHandler
This PR updates all IconComponent usages to use the icon partials in the SDK It also points the test-site to the feature/develop-i18n branch of the SDK, which is the same commit that develop is currently at, but includes i18n assets so our spanish snapshots work The first group of commits do a find and replace for identical (including whitespace like indentation) usages of IconComponent. J=SLAP-1297 TEST=manual test that custom icons can still be set for card CTAs, section titles, and the searchbar see that built in icons still show up
…th (#873) Noticed this while testing replacing all the IconComponent usages. Relative path was not being appended to the iconUrl correctly because the template was referencing just `relativePath` and not `@root.relativePath` or `../../relativePath` (i.e. it was using the wrong handlebars context). This wouldn't have caused visual issues for most users, since with the relativePath not being set correctly the iconUrl would get set to `/<iconUrl>`, instead of something like `../<iconUrl>` which for most cases will point to the same location. This only matters if somebody is taking the files output by the theme's build, and moving them into a directory structure. In that case things may break, since the two iconUrls above may no longer point to the same location. I changed the other two relativePath usages for consistency J=SLAP-1297 TEST=manual test that the iconUrl for alternativeverticals on a full page map now uses the correct relative url, instead of the absolute url that would result from relativePath being null added a percy snapshot for alternative verticals on the full page map, though this doesn't really test what was being fixed
- add opacity setting in theme css - add default loadingIndicator to true in searchbar Note: loading indicator changes require corresponding loading indicator updates from SDK J=SLAP-1335 TEST=manual - Loading indicator is present when set to true during loading - During loading, universal result section have opacity set to 0.5 and 1. Confirm visual update on page
Add a iframe-messaging script partial which can be used to send messages to the parent site in iframe integrations The iframe-common.js script must be overridden in order to listen to these queryId messages. J=SLAP-1439 TEST=manual Override the iframe-common script and print all messages received by the parent frame. Observe a message being received on every search which includes the queryId.
Support i18n versions of the answers-search-ui when specifying the develop branch As part of yext/answers-search-ui#1459, develop now builds i18n assets. When specifying develop however, i18n assets weren't being used because the script partial wasn't including the locale in the asset URL. This PR adds support for that. J=none TEST=manual, auto Build the test site and see that the Spanish site correctly loads the Spanish answers assets from canary. Add test coverage
Listen to runtime config updates for 'querySource' and set the corresponding method in ANSWERS. The theme must be pointed to an SDK version which includes this change for this to work: yext/answers-search-ui#1464. J=SLAP-1438 TEST=manual Test setting the query source in iframe and non-iframe experiences with runtime config and observe the correct source being sent to the API
Update cafeTest acceptance test - Added `waitOnSearchComplete` in verticalResults.js to wait till query responses return status 200 and searchComplete state appears on page. - Register requestLogger to each test to track vertical request and response to Answers API endpoint - Added IE11nocachehook from SDK - From oliver's acceptance test failure investigation..   - first point: the test first function call is `verticalResults.scrollToBottom()`, which wait on `this._resultsWrapper` when perform scroll on page. This resultsWrapper will always exist because it's set in our page template. The test didn't account for waiting on the actual search result div. Calling `waitOnSearchComplete` before scrolling would prevent this issue.   - second point: removed the arbitrary wait time 1s in `dragLeft()`. Replaced with waiting for the response to be returned first using searchComplete state. - Updated tests to wait for results before calling `getNumResults()` which check if results exists after a search. (From testCafe documentation, Selector wait/timeouts functionality have no effect on Selector.exists and Selector.count properties. These properties are evaluated immediately.) J=SLAP-1425 TEST= auto ran testCafe locally to verify tests and wait time in selectors
async function `isLoggerResultsPresent` returns a promise<boolean> on query response, added the missing `await` keyword when invoking the function.
send the sessionId in the same message that the queryId is being sent J=SLAP-1450 TEST=manual Launched iframe test site, performed a search and verified through console log in iframe-common.js onMessage() that a message is delivered to parent page with queryId and sessionId.
- update hook registration to use `requestHook` on fixture instead of `beforeEach` for each test - update waitOnSearchComplete to use current testInstance from test suite instead of the imported testInstance reference from testcafe - update tests on searchThisArea functionality and drag on search
This commit removes the html-asset-loader, which was used to automagically perform webpack loads for static assets. As we have continued to add more complexity to the theme, the scope of the html-asset-loader has grown to a difficult to maintain state. The loader has to search through not only HTML, but also all javascript strings we provide. Furthermore, the implementation of this loader is dependent on implementation specific details of the html-loader. Finally, removing this loader will make it much easier to migrate to ESBuild, or any other bundler we choose. I moved html-loader to the prod build only, and turned on all the default minification settings instead of our custom ones. I also updated the version to the latest. Also update browserlist (`npx browserslist@latest --update-db`) because it was asking to be updated. J=SLAP-1419 TEST=manual see that the test-site still displays static image assets correctly
We want to be able to get translations from smartling instead of asking a hitchhiker/native speaker at yext. Eventually we will probably want some way of injecting translations into the formatter. Also fix rename open-status-18n to open-status-i18n (year old typo). J=SLAP-1455 TEST=none
This commit replaces file-loader with webpack asset modules, which is the webpack 5 way to do it. This is slightly more streamlined than using loaders that are separate npm packages, which have been deprecated by asset modules. This commit also fixes font preloads no longer pointing correctly to font assets. Originally, I tried to separate that out to another PR, however it seems like html-loader@2 only works properly with asset modules and not file-loader. As opposed to downgrading html-loader, then reupgrading it in this PR, they're in the same PR. I removed the minimize=true config for html-loader, because by default html-loader will minimize when mode="production" and won't minimize when mode="development" J=SLAP-1084,SLAP-1419 TEST=manual see that html is minified in prod mode, but not for dev mode see that the font preloads get their URLs updated correctly, for example pointing to source-sans.[hash].woff instead of static/assets/fonts/source-sans.woff see that bundle.css points to the correct URL, and that the font shows up properly on the page
Fix shifting of the voice icon which occurs when clicking the clear button J=SLAP-1473 TEST=manual Click the clear button and confirm the icon no longer shifts. Test with the default voice icon SVGs and with custom images
Fix the grunt memory leak by isolating the jambo and webpack build into their own process J=SLAP-1484 TEST=manual Run "grunt watch" and confirm the build runs when files change
Add a patch file for fixing the memory leak The grunt file hasn't changed in a while so this should work back to v1.17 J=SLAP-1484 TEST=manual Build a site on SGS and apply the patch file. Observe a consistent build time upon file changes
### Bug Fixes - Fixed the logic of the `getLanguageForProvider` utility method. This method maps the `locale` specified by the user to one that is understood by the chosen map provider. (#891) - Corrected a styling issue with the `SpellCheck` query suggestion. This issue made it difficult to click the query suggestion and fire off a new query. (#892)
…ade (#913) This folder is not needed by hitchhikers, and will slightly slow down the build when jambo tries to register all files in themes/[theme] as partials. Previously, there was a bug introduced in #441 where an async method was not being awaited, causing the tests folder to not be deleted. J=SLAP-1452 TEST=manual test upgrading yanswers to this branch, and see that there's no test-site or tests folder
J=SLAP-1516 TEST=manual see that opacity style is used when search Co-authored-by: Yen Truong <ytruong@yext.com>
Use the new partials param in templatedatavalidator instead of fs.existsSync. This is slightly more robust, for weird cases when somebody registers a card or universalsectiontemplate in an atypical folder (for example consulting). J=SLAP-1489 TEST=manual test that the validator will catch incorrect card and unviersalsectiontemplates
#919) J=SLAP-1464 TEST=none
This comes from a QA item where, after upgrading the theme, the global config had duplicated "commented out" props. For example a commented out apiKey like `// apiKey: <REPLACE_ME>` The way comment-json works is that each comment is associated with a specific "prop" of the json (unless the json is empty or the comment is outside of the main json body). If we're not careful with how we add new config to the global_config, the prop associated with the giant commented out config block will change. Because we merge the new theme's global_config with the user's current global_config on upgrade (with the user's having higher priority), this results in said huge block being duplicated, because comment-json sees it as a *different* comment than the preexisting giant one, since it is now associated with a different prop. Update comment-json to latest for small parsing improvements. There are no breaking changes we're worried about. J=SLAP-1514 TEST=manual, auto unit tests test upgrading yanswers, also with some minor tweaks to their config to test things
* Add translations needed for the Summer Release. This PR adds a number of new language files to the Theme: Chinese (Traditional), Chinese (Simplified), Russian, Polish, Portugese, Dutch, Arabic, Korean, Swedish, and Hindi. Exisitng language files were also updated since there were some new strings needing translation. J=SLAP-1461 TEST=none * Fix existing translations.
* Fix inverted check mark in map J=SLAP-1519 TEST=manual see that check mark is not inverted in full page map from en and ar locale
- use ltr instead of rtl attribute in the map div since switching to rtl would disrupt the coordinate to css mapping calculated by the map providers - adjust map boundaries based on location of the result content wrapper, which could be left or right of the viewpoint (ltr or rtl) J=SLAP-1515 TEST=manual - launched multilang site, see that pins on map on arabic is present and the pin locations matched with the ones on english page.
update pin svg to use labelColor from config J=SLAP-1521 TEST=manual change colors in map page config and see that the pin's number color is updated on the page.
* rename chinese po files to use Hans and Hant J=SLAP-1530 TEST=manual build a chinese site * chinese renames for open-status and .po files Renames to zh-Hans and zh-Hant. J=SLAP-1529,1530 TEST=manual tested zh-Hans-cn, zh-Hans, and zh-Hant and saw that both the open status formatter and view all button were translated * revert open status
Support additional languages for the distance formatters Rather than enumerating each locale, I opted to make the unit map only contain locales with imperial units since everything else will default to metric J=SLAP-1529 TEST=auto Run tests
- add table header translations for new languages - move the code to extract language from locale to utils TEST=none
Use language modifiers rather than the region for determining the language script J=SLAP-1529 TEST=manual Build the sample site with both traditional and simplified Chinese. See that the open status strings are translated and different between the two
J=SLAP-1533 TEST=none
- add support for chinese language locale in google map and mapbox - update jest test J=SLAP-1527 TEST=auto jest test
Point to the latest SDK v1.10 J=none TEST=manual Smoke test with the test site
Update Jambo to v1.12.0 J=none TEST=manual Smoke test the test site
tmeyer2115
approved these changes
Aug 24, 2021
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.
Features
Enhancements
Bugfixes