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

Resolve settings scope passed to components #1435

Merged
merged 2 commits into from
Jan 31, 2019

Conversation

mattcoy-arcticleaf
Copy link
Contributor

What?

These changes correct the scope of the settings object in several files.

In this commit, the store settings object was improperly scoped using '../' unnecessarily in some instances. This causes defects in the quick view modal and poses risks for future development. Anything that can be toggled in the store settings does not appear in the quick view modal. This includes the product rating, social share links, and add to wish list button.

To replicate, go to the Cornerstone demo theme, and quick view any product.

Tickets / Documentation

N/A

Screenshots

Before:

before

After:

after

In various component includes, the store settings is incorrectly scoped. These changes correct the scope.
@bigbot
Copy link

bigbot commented Jan 25, 2019

Autotagging @bigcommerce/storefront-team @davidchin

@lord2800
Copy link
Contributor

This explains a bug we just discovered with data tags as well. cc @Ubersmake @bc-ranji @bc-krishsenthilraj

Copy link
Contributor

@junedkazi junedkazi left a comment

Choose a reason for hiding this comment

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

@mattcoy-arcticleaf the changes look good to me. Can you please add a changelog entry.

@mattcoy-arcticleaf
Copy link
Contributor Author

@junedkazi I have added a changelog entry.

@Ubersmake Ubersmake merged commit 2728920 into bigcommerce:master Jan 31, 2019
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

5 participants