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

Stand-alone SearchBar. #1516

Merged
merged 7 commits into from
Aug 18, 2021
Merged

Conversation

tmeyer2115
Copy link
Collaborator

@tmeyer2115 tmeyer2115 commented Aug 18, 2021

This PR adds a new entry point to the SDK's source. This entry point defines a class, AnswersSearchBar, that powers a SearchBar only integration. The class is defined in answers-search-bar.js, which bears many similarities to answers-umd.js. There are a few key differences:

  • The FilterNodeFactory, formatRichText, and processTranslations methods are not exported on the top-level object. These are not needed for this type of integration.
  • We will not handle CSS variables in this integration. Styling is very limited and prescriptive. So, all related ponyfills were removed.
  • It is assumed that the client has pulled down the TemplateBundle on their own and we will not need to load it for them. So, the DefaultTemplateLoader is not used.

A new registry of Components, specific to this type of integration, was added. This registry was put in a new file, search-bar-only-registry.js. At first, I attempted to export a second registry from the existing registry.js. But, Gulp's tree-shaking was still including all Component classes in the SearchBar-only bundle. Creating a new file solved that issue.

Finally, the ComponentManager class is no longer a singleton. This is because it can now accept different componentRegistrys. I don't think this should cause an issue. If you attempt to init twice on your page, you would
already get the same componentManager since ANSWERS itself is a singleton. Likewise if you tried to make multiple instances of either on your page.

The Gulp infrastructure of the SDK was modified to support building the assets for this integration. There is a new top-level command, npm run build-search-bar-only. The existing npm run build, npm run build-languages, and npm run build-locales commands work as before. They do not produce the assets for this integration. The build-search-bar-only command has not been added to the CI either. We still need to determine what the automated build/release process for this integration looks like. It is versioned separately from the SDK. One thing to note is that Product asked the same asset names (answers.js, answers.min.js, answers.css, etc.) be used for this integration.

J=SLAP-1271
TEST=auto,manual

Built a new JS and TemplateBundle asset from this entry point. Verified the following:

  • The SearchBar appears on the page and works as expected.
  • The Autocomplete works as expected.
  • The setContext method could add a context to the search.
  • The redirectUrl worked as expected and the correct query params were added to it.
  • The defaultInitialSearch worked as expected.
  • I could configure the SearchBar component.
  • The setSessionsOptIn method worked as expected.

A new acceptance test was added for this integration. I verified it passed when the integration's assets were present in dist. Note that the test is not run by default since we do not build this integration's assets by default.

@tmeyer2115 tmeyer2115 added the WIP Work in progress label Aug 18, 2021
@tmeyer2115 tmeyer2115 removed the WIP Work in progress label Aug 18, 2021
@@ -1,5 +1,5 @@
{
"src": "tests/acceptance/acceptancesuites/*.js",
"src": ["tests/acceptance/acceptancesuites/*.js", "!tests/acceptance/acceptancesuites/searchbaronlysuite.js"],
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this code has been approved already, but would it make sense to put the searchbaronlysuite into its own folder? Really nitpicking here but I could see a new person inheriting the SDK, and being confused why only some of the acceptance tests would run by default

@tmeyer2115 tmeyer2115 merged commit 126897b into develop Aug 18, 2021
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