-
Notifications
You must be signed in to change notification settings - Fork 83
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
feat: wishlist sharing #1590
base: develop
Are you sure you want to change the base?
feat: wishlist sharing #1590
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- replace the feature toggle by a server configuration attribute or remove it
- move your code to the wishlist extension if possible
...app/extensions/wishlists/shared/wishlist-sharing-dialog/wishlist-sharing-dialog.component.ts
Outdated
Show resolved
Hide resolved
src/app/extensions/wishlists/models/wishlist-sharing/wishlist-sharing.model.ts
Outdated
Show resolved
Hide resolved
src/app/extensions/wishlists/pages/wishlist/wishlist-page.component.ts
Outdated
Show resolved
Hide resolved
src/app/extensions/wishlists/store/wishlist/wishlist.reducer.ts
Outdated
Show resolved
Hide resolved
...extensions/wishlists/pages/account-wishlist-detail/account-wishlist-detail-page.component.ts
Outdated
Show resolved
Hide resolved
src/app/extensions/wishlists/pages/wishlist/wishlist-line-item/wishlist-line-item.component.ts
Outdated
Show resolved
Hide resolved
src/app/extensions/wishlists/pages/wishlist/wishlist-page.component.html
Outdated
Show resolved
Hide resolved
src/app/extensions/wishlists/pages/wishlist/wishlist-page.component.html
Outdated
Show resolved
Hide resolved
...xtensions/wishlists/shared/wishlist-sharing-dialog/wishlist-sharing-dialog.component.spec.ts
Show resolved
Hide resolved
104422d
to
bfc4abd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- please check mobil view
- Shared wish lists could be labeled in the wish list overview. Example:
- if list is not shared anymore and the other user is logged out and opens the URL: empty page with error message "not found" is visible --> page not found page would be better
- if list is not shared anymore and the other user is logged in and opens the URL: wishlist is visible and will be updated, error message can be ignored by the user --> page not found should be visible
See here: https://pwa-gh-review-feature-wishlist-sharing.azurewebsites.net/b2c/wishlists/upxA8AFl0M4AAAGSdHoADkKl?owner=7UFA8AVnAXgAAAGRlpgADVn4&secureCode=408ac0b1-c770-4830-9046-e672822dce84 - 2 requests and responds are sent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls let's discuss ui fixes before you start to work on it
Ami will review French, so please do not merge beforehand. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Localization has been approved (including French checked by Ami). Please let us know if you make any later changes to the wording. Then we will have another look.
- floating buttons changed to normal icon buttons and tooltip with text added - added a label in the wishlist list to indicate that the wishlist is shared - fixed the problems when the wishlist is no longer shared -> redirect to error page - fixed the problem with duplicate requests - added a separate state for the shared wishlist so that logged in users can use the link to the shared wishlist
Azure Demo Servers are available: |
- fix padding left for shared and preferred - button group is in one line with the heading - use the base url for link, but consider multi-channel baseHref configurations - fix e2e test
Azure Demo Servers are available: |
Azure Demo Servers are available: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Design issues are fixed. Well done!
@SGrueber follwing problem still exists:
I share my wish list to a friend. The friend can open it via link. I unshare my wish list. My friend can open it again with the link. If the friend uses another browser, the error page appears (this is correct). It looks like a caching problem. Could you check this, please?
PR Type
[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no API changes)
[ ] Build-related changes
[ ] CI-related changes
[ ] Documentation content changes
[ ] Application / infrastructure changes
[ ] Other:
What Is the Current Behavior?
The wishlist sharing feature is missing in the PWA.
What Is the New Behavior?
The wishlist sharing (B2C only) functionality is now added to the PWA
Does this PR Introduce a Breaking Change?
[ ] Yes
[x] No
Other Information
requires ICM-AS 11.10.0
AB#94495