Version 1.25.0 #979
Merged
Version 1.25.0 #979
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
Merge master (v1.24.0) into develop
- SDK GoogleMapMarkerConfig expects label to be of type string J=SLAP-655 TEST=manual see that map is working properly and error is gone in browser inspector
Improve performance for map pages on mobile by only render result cards on list view - VerticalFullPageMapOrchestrator use a field in core storage `DISABLE_RENDER_RESULTS` to communicate with VerticalResults component whether it should render the results on page. - VerticalResults component's config pass in a beforeMountOverride function to avoid mount() being call in the component lifecycle and render if `DISABLE_RENDER_RESULTS` is true. - VerticalResults component's config also pass in a onCreate function to add a listener on any changes `DISABLE_RENDER_RESULTS`, to potentially trigger render update for the rsults if user switch from map view to list view, or screen resize between mobile and desktop view - Since VerticalResults now will not render any location cards on map view, VerticalFullPageOrchestrator can no longer copy the card and display that when a pin is clicked. So,cardType is passed into the config for VerticalFullPageOrchestrator. When a pin is click, the cardId is used to pull the corresponding entity data from results in storage, and construct a card component based on cardType to append to the top level map container. Note: require update to SDK's component.js file for beforeMountOverride to work J=SLAP-1302 TEST=manual & auto smoke test from theme's locations_full_page_map vertical page: - resize screen from mobile to desktop and vice versa, see that results and focused pins are still loaded properly. - toggle between map and list view, see that the results are only render on list view in elemnt inspection (except for the first time when search is trigger from searchbar) - in map view, drag across the map a few times and see that pins are loaded correctly but results are not render on the page. - see that detail card is displayed correctly when click on pin from map view, before and after dragging acrross the map recorded performance metric on browser inspector, see that the tasks to re-render the components shrink to around ~≤27ms compare to previous time depending on result count (~≥100ms for 2000+ results) (got rid of most of the 'long tasks' - where the main UI thread is busy for 50 ms or longer) no changes in map related snapshots from percy
- when checking usage of window.alert for theme, this is the only place that uses it. But since this class/file is not being used anywhere, we should remove it.
Int.FormatNumber and toLocaleString would fail on locale with underscore, such as chinese locale like 'zh-Hant_CN'. Update function to replace underscore with dash for locale TEST=auto pass updated jest test
- add github action to create pr from master/main to develop on push to master/main (yml file from generator-slapshot repo) J=SLAP-1357
Prior to this change, all errors that occurred during ANSWERS.onReady were being swallowed up by the .catch(). This could lead to very frustrating debugging experiences, and also hide critical errors like a null pointer error. J=SLAP-1616 TEST=manual see that errors are no longer hidden
Add `verticalLimit` and `universalLimit` to vertical-grid's page-config.json, and have them commented out as default. J=SLAP-1618 TEST=none
J=TECHOPS-2183 TEST=manual tested using my local JAMBO_INJECTED DATA tested that errors are emitted when using the deprecated injected api key, but the build continues tested that the build throws an error when no production or deprecated api key exist
Comment out vertical icons per the HH's request With this change, Icons will no longer appear next to the names of verticals on the universal page by default J=SLAP-1619 TEST=manual, visual Build the local test site and see that icons are no longer there except for where a custom icon url is supplied
J=SLAP-1631 TEST=manual,auto saw facets were sorted on the people page
Add a runtime config listener for changes to `visitor`. J=SLAP-1621 TEST=auto, manual Add jest test for visitor listener and check in test-site if `visitor` is sent in analytics events and searches in standard and iframe integration.
Originally, I thought running a toString() on the displayName would handle number display names. But, string.prototype.localeCompare doesn't work with number strings the way I hoped it would. If you try to sort an array of numbers like [100, 20, 120], the result will be [100, 120, 20] rather than the expected [20, 100, 120]. My unit tests only had single digit numbers, so they didn't run test that case. TEST=auto J=SLAP-1631
Add generic thumbs up/down feedback buttons and analytics to all cards (#938, #939, #940). - Include a `feedback` config option in `dataForRender` so user can specify if feedback buttons should appear on the card or not. - Create a shared partial for the buttons that is used in all cards, including direct answer cards - Update analytics for direct answer feedback with additional event attributes. J=SLAP-1544, 1545 TEST=manual Smoke testing to see if thumbs up/down buttons function as expected on all cards and send analytics.
I changed the sorting to not mutate the original array. Originally I was worried about performance, but it should be very minor. J=SLAP-1632 TEST=manual,auto added optionsOrderList to the people page's config override for percy snapshots manually built the page and saw the facets being sorted
The backend does not natively support facets on number fields. The closest thing that is supported is assuming a string field contains a parseable number. This PR removes support for interpreting string fields as numbers, to be consistent with the backend. J=SLAP-1631 TEST=manual rebuilt page and checked optionsOrder and optionsOrderList facets
Use the color-text-primary variable for the color of text on the body of cards This change makes most text slightly lighter because the value of --yxt-color-text-primary is `#212121;`. Previously the cards were falling back to the default of the browser which is black. The item mentioned using the text mixin to accomplish this, however I decided to use the variable directly because there is less chance of side effects because the text mixin affects things other than color. J=SLAP-1617 TEST=manual For testing purposes, set the text to red and view all of the cards of the test site. See that the color of the body of text is red as expected.
Only show thumbs up/down feedback buttons when user provides `businessId` and enables analytics. J=SLAP-1175 TEST=manual Smoke testing to check if feedback buttons only appear when expected.
This reverts commit d9bf111.
Revert "Add visitor support"
This reverts commit 646c166.
This reverts commit d3d591b.
Only show thumbs up/down feedback buttons on direct answer cards when user provides `businessId` and enables analytics. J=SLAP-1175 TEST=manual Smoke testing to check if feedback buttons only appear on DA cards when expected.
tmeyer2115
reviewed
Oct 13, 2021
Yep, will do! |
Reverts #955 because the necessary corresponding [PR in the SDK](yext/answers-search-ui#1548) didn't make it into v1.11 J=none TEST=manual Build the test site with the new v1.11 SDK and see that the icons now look correct
Update to Jambo v1.12.1 and SDK v1.11
nmanu1
reviewed
Oct 13, 2021
good catch! updated |
yen-tt
approved these changes
Oct 13, 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.
Version 1.25.0
Features
Enhancements
Bug Fixes