v1.28 #1043
Merged
v1.28 #1043
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
Update ThemeMap to support coordinate and pin constructions based on data from job entity type.
- update transformDataToUniversalData and transformDataToVerticalData to guard access to countryCode field since job entity will not have the address field like location entity.
- update ThemeMap to NOT construct pin if entity doesn't have `entity.profile.yextDisplayCoordinate`.
- Note: job entity may include only mapMarker field without a location field. This would not give us `entity.profile.yextDisplayCoordinate` but instead in `entity.profile.displayCoordinate`. Product only wants display from `entity.profile.yextDisplayCoordinate` for now.
- move cardFocusUpdateListener up to make sure selecting cards that doesn't have pin would still deselect previously selected pin
J=SLAP-1744
TEST=manual
Added some more job entities into knowledge graph: some with location field, some with map marker, some without any location or map marker:
- test with `locations_full_page_map` page with verticalKey: 'jobs' and cardName: 'location-standard'
- See no errors in console.
- Click on cards and pins. See that cards with corresponding pins work as normal. See that cards without pins would still highlight the card but unhighlight the previous selected pin.
- Drag map arounds and see that only cards with pins shows up when rerender
- modify pin to display index number, see that the index displayed is correct corresponding to the ordinal number of the cards
- test with `locations_full_page_map` page with verticalKey: 'jobs' and cardName: 'job-standard'
- see no errors in console.
- pins still work as expected
- test with `locations_full_page_map` page with verticalKey: 'KM' and cardName: 'location-standard'
- smoke test the page with location entities to confirm that it still work as expected with clusters and non clusters pins
- test with job entities on other pages:
- `locations_full_page_map_with_filters`: no errors, pins and cards show up and can be selected as expected
- `locations_google` and `locations` (uses map from SDK): no errors, pins and cards show up
- NOTE: SDK map component currently will NOT have the right the pin ordering with the card ordinal numbers if there are cards created from job entity without location field. This will be investigated in another item
Improve Acceptance Test Speed The primary reason for the slowdown is that Browserstack will run IE11 tests across many browserstack sessions. The firefox and safari tests wait for each test to pass in IE11, which is why there are long periodes of time where nothing is happening in the safari and firefox test videos. I am submitting a support ticket to browserstack to see if they can explain why this happens for IE11. Separating the tests into their own jobs prevents the safari and firefox tests from waiting for the IE11 video, which makes the browserstack videos much more useful. For now, increasing the concurrency of IE11 runs helps alleviate this issue. I don't recommend we run IE11 with a concurrency less than 3 because it improves the test runtime to about 7 from 20-25. This puts us at 5 concurrent tests for non-dev branches. If Browserstack gets back with a fix, then we can decrease or remove the IE11 concurrency. So far we haven't ran into issues with too many running tests, but if it's a problem then we can consider decreasing the concurrency or not testing IE11 on so many branches. Decreasing the zoom stabilization interval helped with performance by a few seconds overall. Finally, I moved all the locator acceptance tests into one page because the request logger wasn't being setup properly with the two separate files. The request logger not working was causing the tests to take an additional 30 seconds because the `waitOnSearchComplete` would wait the full 10 seconds before proceeding. The issue was that `SearchRequestLogger.createVerticalSearchLogger()` was being called twice which created two separate `RequestLoggers` rather than one. When I reuse the same request logger across both test fixtures, the issue is resolved. The browser in safari would sometimes not be wide enough for the full page map tests, so I set a fixed value which keeps the map in desktop mode. J=1756 TEST=auto Confirm tests all still pass See that browserstack test speed improves from 1min 15seconds to about 45 seconds locally. See that browserstack speed improves from over 20 minutes to about 7.5 minutes for IE11.
Set the default to off for map zoom while scrolling for maps on the universal search page. This was a problem for MapBox maps, because scrolling on the universal page would scroll in the map when the cursor was in the map, which was not ideal behavior. Note: a [PR](yext/answers-search-ui#1648) in the SDK added zoom control buttons for MapBox by default so that the map can still be zoomed if desired. J=SLAP-1765 TEST=manual Check that scrolling does not zoom the map on universal pages, but vertical pages are unchanged. Check that this can be switched back on if `scrollZoom` is set to true in the config
This commit refactors how global configs are merged on theme upgrades. Now, global configs will always have the same "structure" as the incoming config, instead of sometimes placing properties in seemingly random spots, if the two configs don't match up exactly in terms of structure. Per Rose's request, the sdkVersion is now always updated to the incoming config's value. Besides that, preexisting properties will favor the original config, while commented out properties will favor the incoming config. `Property`s and `CommentedOutProperty`s in the original that do not exist in the incoming config will be moved to the bottom of the config. Comments that are neither inline nor `CommentedOutProperty`s will be ignored gracefully, and an error message displayed. J=SLAP-1717 TEST=manual,auto ran an upgrade from 1.26 to this branch ran an upgrade from 1.26 to this branch, but with the global config changed to have all values replaced with new ones, and with an extra prop at the bottom
close autocomplete on click in parent window by sending message from parent window to iframe using iFrameResizer library in iframe-common.js, run by parent page, add an event listener to the document for click event. When trigger, check if the event is trusted (invoke by user, not external script) and send a message to iframe. In html.hbs, run by iframe, modify onMessage function in iframeResizer to check for message that contains event type 'click'. If so, get all active autocomplete components and invoke their close function. J=SLAP-1182 TEST=manual host a site in SGS with a page that uses iframe.js from localhost port (with the pr changes) for cross-origin test. Also modify page to have multiple search bar. See that autocomplete closes when click outside of iframe.
Integrate with Yext Sites Add config to support yext sites. The main "gotcha" is that Sites renames the answers-hitchhiker-theme to 'repo', so I added an extra jambo config which we can use with Yext Sites which refers the the theme as 'repo'. Once we merge this into develop and master, we should be able to see those sites hosted at: - theme.slapshot.pagescdn.com - https://develop-theme-slapshot-pagescdn-com.preview.pagescdn.com/ The live preview integration is a little out of scope, but I think it would be useful if we add it in the future. I ran into an issue with it which may also have to do with repo renaming. It would allow us to open up the test site for any PR that we push to Github which would be useful for testing J=SLAP-1679 TEST=manual Connect this branch to Yext Sites and see the site hosted at https://devserveonyextsites-theme-slapshot-pagescdn-com.preview.pagescdn.com/
- Add rtf styling to event-standard component - move rtf styling from `&-details` to `&-detailsText` for faq-accordion and location-standard component. J=SLAP-1804 TEST=manual test event page, location_google page, and faq page with rich text description. See that the text is styled properly with rtf styling.
this pr resolve an issue that occur when there's a conflict between custom/new card name and the original card component name. Specifically, if the new card name contains the original card name, the series of replace functions invoked in `_getRenamedCardComponent` would not produce the correct file content (i.e. old name: 'standard', new name: 'standard-override'). This occurs in card and directanswercard command. Fix: Update the series of replace functions in `_getRenamedCardComponent` to first replace all original component name with custom card name (without the underscore replacement), then update the two spots in the file that would require the custom card name with underscore replacement. J=SLAP-1800 TEST=manual cd into test-site and run `npx jambo [directanswer]card --name xxx --templateCardFolder [directanswer]cards/xxx` with standard-override, documentstandard-override, event-custom2, product-standard-override, etc. See that component.js have correct class names. Used those cards in test site, see that pages work as expected.
Change the default `universalSectionTemplate` for vertical-grid to grid-three-columns. J=SLAP-1831 TEST=manual Check that the test-site shows the correct layout on the universal page for the people vertical.
Add a multi-lang document standard card. Update help_articles.es.json to use this card for the test-site. J=SLAP-1839 TEST=manual Check that help articles on the es test-site use the new card and show translated text for feedback submission.
Add a multi-lang document search DA card. Update index.es.json and index.ar.json to use this card for featured snippets in the test-site. Fix a bug in the allfields DA card template that affected the styling for the title when there is an icon. J=SLAP-1830 TEST=manual Check that the doc search card has translated feedback text and that the allfields DA card title is styled correctly with an icon.
Merge master (v1.27.0) into develop
|
For "Previously, if there was a Map on the Universal page, a user could get stuck on the Map while scrolling down the page. This has been fixed" I think we want to add #1030 to the PR list as it's the original PR I would add that we now automatically bump a global_config's |
|
Do we want to say anything about #1029? I'm not super familiar with this one |
I left that out because I didn't see it called out in the Product 1.28 announcement. |
oshi97
approved these changes
Feb 8, 2022
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
document-standardresult card and thedocumentsearch-standarddirect answer card have been added. (Add multilang doc search DA card #1041, Add multilang doc standard card #1039)sdkVersionin theglobal_configon upgrade. (refactor global config merging strategy on upgrade #1031)Changes
event-standard,faq-accordion, andlocation-standardcards. (Add missing rtf styling in components #1037)universalSectionTemplatefor avertical-gridpage was updated togrid-three-columns. (Update vertical-grid universalSectionTemplate #1038)Bugfixes
cardcommand. If the new card's name contained the name of the forked card, the former would not be added properly. This is now fixed. (Fix jambo add card command #1036)