-
Notifications
You must be signed in to change notification settings - Fork 386
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: Use tabs instead of radio group for pickup options #19406
base: develop
Are you sure you want to change the base?
Conversation
@@ -89,4 +142,46 @@ export class PickupOptionsComponent implements OnChanges { | |||
// Return false to stop `onPickupOptionChange` being called after this | |||
return false; | |||
} | |||
|
|||
protected initializeTabs() { | |||
this.tabs = [ |
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.
@Zeyber here I've tried to use the tabs$
input property here, however it didn't work for me. We can catch up on this issue on slack
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.
I can see there is a lot going on with this component with pre-existing change detection calls and emits. I don't think the TabComponent
can really be designed to account for it. So, the way you update the tabs based on this component's logic is ok :)
Merge Checks Failed
|
32d2719
to
35ee2bc
Compare
Merge Checks Failed
|
35ee2bc
to
6cf0293
Compare
Cherry-picked commit from #19421 to fix the unit test failures |
5e026ad
to
265b876
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.
I am happy to see that we will be able to clean up a lot of the complexity in future :)
Only minor comments from me. But lets wait to merge this since the TabComponent logic overlaps.
...re-libs/pickup-in-store/components/presentational/pickup-options/pickup-options.component.ts
Outdated
Show resolved
Hide resolved
...re-libs/pickup-in-store/components/presentational/pickup-options/pickup-options.component.ts
Outdated
Show resolved
Hide resolved
@@ -89,4 +142,46 @@ export class PickupOptionsComponent implements OnChanges { | |||
// Return false to stop `onPickupOptionChange` being called after this | |||
return false; | |||
} | |||
|
|||
protected initializeTabs() { | |||
this.tabs = [ |
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.
I can see there is a lot going on with this component with pre-existing change detection calls and emits. I don't think the TabComponent
can really be designed to account for it. So, the way you update the tabs based on this component's logic is ok :)
Merge Checks Failed
|
265b876
to
5381ad8
Compare
Merge Checks Failed
|
Closes: https://jira.tools.sap/browse/CXSPA-7972