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

v1.17.0 #521

Merged
merged 39 commits into from
Dec 1, 2020
Merged

v1.17.0 #521

merged 39 commits into from
Dec 1, 2020

Conversation

tmeyer2115
Copy link
Collaborator

Version 1.17.0

Version 1.17.0 of the Theme supports the new Overlay integration. It also has a number of changes to support the new, built-in Product entity type.

Changes

Bug Fixes

alexisgrow and others added 30 commits November 17, 2020 17:19
Per this PR: yext/answers#1168, the hook "onSubmit" was renamed to "onConductSearch". There was also a bug that was always using the button's max-width (100%) rather than the default button width when no text was present.

TEST=manual

Test locally using a newly generated HH test site with the theme and v1.7 of the Answers SDK. Serve, conduct search via buttons and via autocomplete.
…481)

Noticed on yanswers that the sticky results button listener was
throwing an error for getBoundingRect() not being found.
This was happening because collapsible filters was looking
for a .js-answersResultsWrapper class, which is not present
on vertical-standard/grid pages, or older vertical-map pages.

Only vertical-map needs the sticky results button listeners,
so the registration of these listeners was moved into the page-setup
of the vertical map page.

This commit also updates cfilters to fall back to .Answers-resultsWrapper,
for easier integration with older page templates. I felt this was necessary
because adding the .js-answersResultsWrapper class was not listed in the
Collapsible Filters integration instructions.

J=SLAP-863
TEST=manual

Tested that the yanswers experience will no longer throw getBoundingClientRect()
not found errors
Some of the CSS resets for body styling from our generic base.scss file were causing issues with the button's styling, so we do not import that file for the button frame any longer. Removed extraneous width CSS that was causing weird width calculation issues as well.

TEST=manual
J=PB-9812

Reproduce issue on Krispy Kreme site - see button fill max-width. Test after change and see button frame is sized based on content.
Previously, the elements within the overlay container had a z-index values but the container itself didn't. This was causing issues where the container could be under other items on the page. Add zindex to the container.

TEST=manual
J=PB-9812

Reproduce issue on Krispy Kreme site - see menu labels appear on top of the overlay. Test after change and see overlay is on top of all other elements.
Default the field value to an empty object to prevent errors at runtime

J=none
Test=manual

Confirm that no error is thrown when the value provided to the formatter is undefined
Default JAMBO_INJECTED_DATA to null

Since Webpack 5, a webpack build will throw an error if an environment variable does not have a value or a default. This causes builds to fail when building locally when JAMBO_INJECTED_DATA is not set. Defaulting to null resolves this issue.

J=none
Test=manual

Build a site locally and confirm that it now builds when JAMBO_INJECTED_DATA is not set
The button resizes from ~64px to a larger size when the overlay expands and collapses. This is because when the overlay is collapsed, there can be label text like "Questions?", but when the overlay is expanded the button is always the same size. When the button is animating between its smaller and larger states, some browsers try to display scroll bars, which we do not want. Hide overflow to avoid seeing scroll bars.

TEST=manual,visual
J=PB-9812

Test on Firefox with button label text, expand and collapse button and see no scroll bars.
Update the colors of the product tag as specified by the QA doc

J=None
Test=Manual

Build a site with products with product tags and confirm the colors are correct
Currently the CSS variable ponyfill lives in the Answers SDK. Since we don't use the Answers SDK in the button iframe, our variables were not being ponyfilled which meant that they weren't working in IE11. Add the ponyfill to the button iframe's JS.

TEST=manual
J=PB-9812

Serve page in IE11, see font family is font family specified in the answers-variables file rather than the default IE11 font.
Recently, the `JAMBO_INJECTED_DATA` was updated so that the `displayName` of
each vertical in an experience is provided. We should default to this name
when constructing the vertical's universal section template. As before, if
a `label` or `secionTitleName` has been specified in page configuration, we
will favor those instead.

J=SLAP-887
TEST=manual

Tested the following:

- Set a `label` for a vertical, verified that was used as the section title.
- Set a `sectionTitleName` for a vertical, verified that was used as the
  section title.
- When no `label` or `sectionTitleName` was present, saw the `displayName` from
  the injected data used as the section title.
- If no `diplsayName` was present for the vertical, saw it's `verticalKey` used
  as the section title.
Tweak the CSS so that long addresses on location cards don't push the hours list below the address

Adding the max-width causes the address to wrap rather than pushing the hours list below the address. The min-width prevents the width of the address from getting too small when the browser window is small but not yet at the mobile breakpoint. I also switched the padding right to margin right because padding right would sometimes create a lot of whitespace when the address wrapped.

J=none
Test=visual

View a location page as I resize it from mobile to full-screen. Test long addresses and confirm they wrap at reasonable spots.
Fix IE11 formatting for the hours list formatter

When a particular day contains a list of hours, the day should appear next to the top of that list. This is default behavior in chrome, but in ie11, it incorrectly appears next to the bottom of that list. This PR makes this explicit in the CSS so that it also works in IE11

J=None
Test=visual

View the hours list formatter on chrome and observe no difference. Also view it on IE11 and see that the day is now in the right spot
Don't show a product tag if there is no image for the product-prominentimage and the product-prominentimage-clickable cards

Before this change, the tag would cover up the card title if no image was present. These cards are not meant to be used without images, however, this change improves the UI in case that someone forgets to add an image.

J=none
Test=manual

Search for some products that don't have images and confirm that the tag doesn't appear unless the product has an image
This commit adds the isAbsoluteUrl helper to determine urls,
and updates the regex we were using to also detect urls of the
 mailto:slapshot@gmail.com. It cherrypicks #475 

TEST=manual

test that I can build a vertical standard page
* Bumps down the jambo ver back to 1.7.0
Doing this removes access to the isAbsoluteUrl helper, so
all usages had to be manually updated to the corresponding
`matches 'absoluteUrlRegex'` usages 

* Fixes a messed up a merge conflict from 1.16.1
The match helper param order was swapped, it should
be regex second. There were also a few urls that
were missing usages of it that needed it, so that was
added in here.

TEST=manual

test that amani's profile is recognized as an absolute url
Since the card templates are compiled by the SDK's handlebars compiler rather than Jambo, they do not have relative path. Added the relative path from Jambo to the state.

TEST=manual

Generate a site and use relativePath in a card, see it is as expected.
Fixes the outdated caniuse error by not printing to stderr when running the grunt jambobuild

J=none
Test=manual

Test the change in SGS and locally
Since the card templates are compiled by the SDK's handlebars compiler rather than Jambo, they do not have relative path. Added the relative path from Jambo to the state.

TEST=manual

Generate a site and use relativePath in a card, see it is as expected.
Don't exit with a non-zero exit code when the jambo build prints to stderr

J=SLAP-899
Test=manual

Run grunt jambobuild with this change and confirm that the build exits with a exit code of zero, even if jambo printed to stderr
Previously we were trying to access the config.button.alignment before we determined whether or not config.button was present. Updated to access alignment after the config.button is null checked.

TEST=manual

Specify no button config, see no JS error about accessing properties of undefined config.button.
There was a chance of the collapsible filters interactions class
trying to access window.parentIFrame before it was initialized.

This commit fixes that by adding an iframeLoaded promise to the
window, which will be waited on before trying to use the
iframe resizers getPageInfo() listener.

TEST=manual

tested that the scroll listener is never called on an iframe page,
and the resize listener is debounced with the iframe getPageInfo listener

tested on both ie11 and chrome, for both iframed and non-iframed pages,
that the sticky view results button works,

tested that even if I delay the stickifyViewResultsButton() call,
the listeners will be registered correctly
Previously, with the "hideDefaultButton" config option set to true, we would conditionally hide or show the button based on the Overlay's expanded/collapsed state. Removing that logic to always hide the button if "hideDefaultButton" is true.

TEST=manual

Use custom selector and set hideDefaultButton to true. Click the custom button selector and see Overlay open and close without button appearing at all.
The offset was on the Answers frame's iframeContainer rather than the parent container. This was previously fine because we didn't have a button iframe in addition to the other frames; but now, the translation has to happen to the parent.

TEST=manual

Test locally with a horizontal offset of -100px, see entire Overlay shift left 100px. Test with a vertical offset of -16px, see Overlay shift to be flush with the bottom of the screen.
Renamed to more closely match CSS convention. Established naming for colors are "color" and "background-color"; since "foreground-color" is not a norm we are removing it in favor of "text".

TEST=manual

Confirm that the color of the text/icon can still be updated with the new config name "text".
cea2aj and others added 9 commits November 30, 2020 19:47
Merge master into release/v1.17 and resolve conflicts.

Also, update the package version numbers and incorporate #503

J=none
Test=none
Updating the caniuse dependency since we are getting a warning that it is out of date. This PR also alphabetizes our package.json.

TEST=manual

Build and serve a site locally with this change, confirm that nothing looks suspiciously broken.
Set a fixed width for the location standard card

This aligns the open status

J=none
Test=visual

View a site using this styling and make sure it looks right at various widths
We're going to delay this fix until 1.18. This reverts #514 and #494

J=none
Test=visual

View an answers site with this change and confirm it looks the same as 1.16
#516)

Certains cards reference {{relativePath}} inside of
an inline partial, like a CTA partial (see the standard card)
Since relativePath is passed into all cards at the root context,
through the SDK's setState(), all cards will have relativePath
as part of their root context.

TEST=manual

in the standard card:
test that I can create a custom card, and add a hardcoded cta
with an iconUrl pointing to a static asset

test that relative links still work in the standard card cta
…ets (#482)

This commit fixes the relative path calculation in the html-asset-loader.
It also adds a webpack alias to the static folder, so that the loader can
use the static asset paths regardless of the relative path of the
html file it's loading. I.e. if es/index.html and index.html
both use a static/assets/images/yext-logo.svg, they
both import them with the same require, 
`require('static/assets/images/yext-logo.svg')`,
instead of one the first being:
`require('../static/assets/images/yext-logo.svg')`,
and the second being:
`require('./static/assets/images/yext-logo.svg')`,

The isAbsoluteUrl helper had to be updated to recognize urls
that only begin with a single slash as absolute urls as well.

J=SLAP-881
T=https://yextops.zendesk.com/agent/tickets/366842
TEST=manual

test that the WHO site can specify an iconUrl that points to a static
asset

tested that a fresh site can also specify an iconUrl in a CTA
that points to a static asset, and also can directly put in
img tags with a static asset src
Add dev dependencies that allow the post upgrade test to run

Also, tweak the test to make it pass

J=none
TEST=none

Run the tests and make sure they all pass
TEST=manual

Serve locally and confirm it was trying to retrieve the https://assets.sitescdn.net/answers/v1.7/answers.min.js asset (and similar for the template bundle & CSS). The asset 403's currently since we haven't cut v1.7.
The hbs helper isAbsoluteUrl was also matching
on urls that only started with a single backslash,
since those don't need {{relativePath}} to be appended
to them. Technically, though, those urls are root-relative urls,
not absolute urls.

TEST=manual

test that a static asset iconUrl in the standard card cta,
and a favicon that points to a static asset, both work
@tmeyer2115 tmeyer2115 merged commit 5bb570c into develop Dec 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.

5 participants