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

PWA-465 pdp implement the image slider #84

Merged

Conversation

devbucket
Copy link
Contributor

No description provided.

@devbucket devbucket added the enhancement New feature or request label Jun 4, 2018
@devbucket devbucket self-assigned this Jun 4, 2018
@@ -3,6 +3,7 @@ import {
routeDidLeave,
} from '@shopgate/pwa-common/streams/history';
import { getRedirectLocation } from '@shopgate/pwa-common/selectors/history';
import setViewTitle from '@shopgate/pwa-common/action-creators/view/setViewTitle';
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use the setTitle action

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -47,6 +49,11 @@ function product(subscribe) {
dispatch(setProductVariantId(null));
}
});

subscribe(productReceived$, ({ dispatch, action }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

The title should only be set if the user is still on the product page with the received id.

import PropTypes from 'prop-types';
import portalCollection from '../../helpers/portals/portalCollection';
import { componentsConfig as config } from '../../helpers/config';

/**
* The Portal component.
*/
class Portal extends Component {
class Portal extends PureComponent {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert

Copy link
Contributor

@richardgorman richardgorman left a comment

Choose a reason for hiding this comment

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

See comments

@@ -239,7 +239,7 @@ const getProductImagesState = state => state.product.imagesByProductId;
* @return {Array|null}
*/
export const getProductImages = createSelector(
getCurrentProductId,
(state, props) => props.productId,
getCurrentBaseProductId,
Copy link
Contributor

Choose a reason for hiding this comment

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

This selector needs to be changed to not rely on a product id stored in Redux

Copy link
Contributor

@richardgorman richardgorman left a comment

Choose a reason for hiding this comment

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

See comments

@richardgorman richardgorman merged commit d62558b into PWA-327-Exchange-Router Jun 8, 2018
@richardgorman richardgorman deleted the PWA-465-PDP-Implement-the-ImageSlider branch June 8, 2018 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants