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

refactor: introduce product context #403

Merged
merged 3 commits into from
Jan 30, 2021
Merged

Conversation

dhhyi
Copy link
Collaborator

@dhhyi dhhyi commented Oct 5, 2020

PR Type

[x] Application / infrastructure changes

What Is the Current Behavior?

Component composition also includes bubbling of data and events that could be handled by contexts for products.

What Is the New Behavior?

Major refactoring:

-> less logic in templates, components

Still open for later:

  • refactor quickorder page
  • promotions, inventory, product-image components could also use context

Does this PR Introduce a Breaking Change?

[x] Yes
[ ] No

@dhhyi dhhyi added the research Exploration for fancy new stuff label Oct 5, 2020
@dhhyi dhhyi self-assigned this Oct 5, 2020
@dhhyi dhhyi force-pushed the explore/product-context branch 2 times, most recently from 5076a6c to d8c8538 Compare October 5, 2020 23:52
@dhhyi dhhyi force-pushed the explore/product-context branch 2 times, most recently from 8460219 to eb1bb13 Compare October 11, 2020 08:20
@dhhyi dhhyi force-pushed the explore/product-context branch from eb1bb13 to 13d5fd6 Compare October 23, 2020 13:50
@dhhyi dhhyi force-pushed the explore/product-context branch 3 times, most recently from 063bd89 to 0e4b507 Compare November 6, 2020 20:48
@dhhyi dhhyi force-pushed the explore/product-context branch 3 times, most recently from 1445d7c to eb07a18 Compare November 17, 2020 10:17
@dhhyi dhhyi force-pushed the explore/product-context branch from 785bfe2 to 20b534d Compare November 20, 2020 10:13
@dhhyi dhhyi force-pushed the explore/product-context branch 2 times, most recently from b8bf25e to 1263cda Compare November 30, 2020 18:41
@dhhyi dhhyi force-pushed the explore/product-context branch 6 times, most recently from 6f554c9 to 6d387f9 Compare December 9, 2020 10:54
@dhhyi dhhyi force-pushed the explore/product-context branch 2 times, most recently from 49b2c1e to 795953c Compare January 4, 2021 16:38
@dhhyi dhhyi force-pushed the explore/product-context branch 3 times, most recently from ad6a6ba to 12ab907 Compare January 5, 2021 22:44
Copy link

@M-Behr M-Behr left a comment

Choose a reason for hiding this comment

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

This is a very cool new quantity component. I like it.

Behavior issues:

  • If you have a product in the cart and you enter 0 in quantity field or use the arrow, the product is removed. The new component displays an error message in this case. It is possible to enter 0 or use the "-" button to remove the product from the cart?
  • The quantity field is very large in phone or tablet view on some pages. It think width: 50% is fine for the smaller breakpoints, where the component has the whole line or we can define a max. width for the component (maybe 165px - component size in product detail page)?
    width in responsive

Some styling issues:

  • The left border of the focus effect is missing in the cart => This only appears in the cart, maybe it is not a component issue?
    focus - left border
  • Space is missing between quantity component and add to cart (wish list page).
    missing space
  • A blank character is missing after product attribute label in the product list component for variation products.
  • The variation product attributes have 8px margin between them. Please remove it, so that the attributes are grouped together.
    variation product - product list
    Note: I saw that the font color of the product variation attributes was changed from 444 to 999. This is fine for @iwiederhold and me.

@dhhyi dhhyi force-pushed the explore/product-context branch from 41eedcb to 26d0385 Compare January 19, 2021 10:31
@dhhyi dhhyi force-pushed the explore/product-context branch 2 times, most recently from 84de78b to 886e6cf Compare January 28, 2021 20:46
@dhhyi dhhyi changed the title explore: product context refactor: introduce product context Jan 29, 2021
dhhyi added a commit that referenced this pull request Jan 29, 2021
BREAKING CHANGE: refactoring the way product specific components retrieve data with context facade
@dhhyi dhhyi force-pushed the explore/product-context branch from 886e6cf to 3f65120 Compare January 29, 2021 10:40
@dhhyi dhhyi changed the base branch from develop to feature/punchout_management January 29, 2021 10:40
@shauke shauke force-pushed the feature/punchout_management branch 2 times, most recently from 6fd14cb to e6ec72d Compare January 29, 2021 12:25
dhhyi added a commit that referenced this pull request Jan 29, 2021
BREAKING CHANGE: refactoring the way product specific components retrieve data with context facade
@dhhyi dhhyi force-pushed the explore/product-context branch from 3f65120 to c1b5ddc Compare January 29, 2021 13:21
dhhyi added a commit to dhhyi/intershop-pwa-testing-workshop that referenced this pull request Jan 29, 2021
BREAKING CHANGE: refactoring the way product specific components retrieve data with context facade
Base automatically changed from feature/punchout_management to develop January 29, 2021 17:49
BREAKING CHANGE: refactoring the way product specific components retrieve data with context facade
@dhhyi dhhyi force-pushed the explore/product-context branch from c1b5ddc to 95c7cca Compare January 29, 2021 17:54
@shauke shauke force-pushed the explore/product-context branch from e669d67 to 24b7ed0 Compare January 30, 2021 17:38
@shauke shauke merged commit 043d2eb into develop Jan 30, 2021
@shauke shauke deleted the explore/product-context branch January 30, 2021 18:19
shauke pushed a commit that referenced this pull request Jan 30, 2021
BREAKING CHANGE: refactoring the way product specific components retrieve data with context facade
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Refactoring of current code research Exploration for fancy new stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Variation Master - Different buttons are displayed on tile view, list view and product detail page
3 participants