Feat: As an assisted service agent, I want to view list of customer coupons for the customer #17748
Conversation
| (click)=" | ||
| refreshCustomerCouponList.emit({ | ||
| assignable: false, | ||
| searchQuery: undefined |
There was a problem hiding this comment.
no need pass searchQuery ？
There was a problem hiding this comment.
I have just split the function, hope now it looks more clear.
In this scenario, user doesn't search anything. so no need to pass searchQuery.
| assignable: undefined, | ||
| searchQuery: searchBox.value | ||
| }); | ||
| searchBox.value = '' |
There was a problem hiding this comment.
why need clear the value of search box when press enter ?
There was a problem hiding this comment.
Some of search box use this behavior. I will check it with UX.
| @Output() apply = new EventEmitter<PromotionListEntry>(); | ||
| @Output() remove = new EventEmitter<PromotionListEntry>(); | ||
| @Output() removeAlert = new EventEmitter(); | ||
| @Output() removeAlertForApplyAction = new EventEmitter(); | ||
| @Output() refreshCustomerCouponList = new EventEmitter<{ | ||
| assignable: boolean | undefined; |
There was a problem hiding this comment.
| assignable: boolean | undefined; | |
| assignable?: boolean; |
There was a problem hiding this comment.
the code has been refactored. Please check the latest code.
| [showRemoveButton]="true" | ||
| [showApplyButton]="true" | ||
| [isCustomerCoupon]="true" | ||
| (refreshCustomerCouponList)="changeTab($event.assignable, $event.searchQuery)" |
There was a problem hiding this comment.
not only changeTab, others like search will emit refreshCustomerCouponList , maybe function name can refine refreshCustomerCouponListHandler ?
There was a problem hiding this comment.
the code has been refactored. Please check the latest code.
| @@ -3,6 +3,61 @@ <h4 class="cx-asm-customer-promotion-listing-heading-text"> | |||
| {{ headerText }} | |||
| </h4> | |||
| </div> | |||
| <ng-container *ngIf="isCustomerCoupon"> | |||
There was a problem hiding this comment.
is more reasonable put these to asm-customer-customer-coupon component ? seems asm-customer-promotion-listing component is a more common and base list component.
There was a problem hiding this comment.
will explain you in slack
There was a problem hiding this comment.
consider to use ng-content
There was a problem hiding this comment.
ng-content works well. Thx a lot!
| } | ||
|
|
||
| public changeTab( | ||
| assignable: boolean | undefined, |
There was a problem hiding this comment.
the code has been refactored. Please check the latest code.
| #searchBox | ||
| class="cx-asm-customer-promotion-listing-search-input" | ||
| placeholder="{{ 'customer360.customerCoupons.searchBox' | cxTranslate }}" | ||
| (keydown.enter)=" |
There was a problem hiding this comment.
if value changed do we need tigger search ? if yes maybe need consider debounce , if not ignore it.
There was a problem hiding this comment.
only click enter or the search button will trigger search.
…to make it more clear
| [showApplyButton]="true" | ||
| [isCustomerCoupon]="true" | ||
| (changeCustomerCouponTab)="changeTab($event)" | ||
| (searchCustomerCoupon)="searchCustomerCoupon($event)" |
There was a problem hiding this comment.
seems no need this two events for tabs and input has put in asm-customer-customer-coupon component
| <cx-icon | ||
| class="success" | ||
| type="SUCCESS" | ||
| *ngIf="!isCustomerCoupon" |
There was a problem hiding this comment.
I see the *ngIf="!isCustomerCoupon" that has been used three times, How about only use one time?
e.g:
<ng-container *ngIf="!isCustomerCoupon">
<td>
<cx-icon class="success" type="SUCCESS"></cx-icon>
</td>
<td class="cx-asm-customer-promotion-listing-applied">
{{ applied }}
</td>
<td
*ngIf="showRemoveButton"
class="cx-asm-customer-promotion-listing-action-separator"
>
|
</td>
</ng-container>
There was a problem hiding this comment.
agree, it will be more clear.
| {{ applied }} | ||
| </td> | ||
| <td | ||
| *ngIf="showRemoveButton" | ||
| *ngIf="showRemoveButton && !isCustomerCoupon" | ||
| class="cx-asm-customer-promotion-listing-action-separator" | ||
| > | ||
| | |
There was a problem hiding this comment.
the | seems like redundant
There was a problem hiding this comment.
it not redundant, another section using this template will need it
| type: Customer360Type.CUSTOMER_COUPON_LIST, | ||
| additionalRequestParameters: { | ||
| assignable: assignable, | ||
| searchQuery: undefined, |
There was a problem hiding this comment.
seem no need assignment undefined to var, Could you remove it ?
|
sonar failed caused by an issue need to be marked. I will invite people who have authority to mark it after finishing code review. |
ticket: https://jira.tools.sap/browse/CXSPA-3945