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

Feat: my account v2 (holidays) #18348

Merged
merged 213 commits into from
Feb 12, 2024
Merged

Feat: my account v2 (holidays) #18348

merged 213 commits into from
Feb 12, 2024

Conversation

i53577
Copy link
Contributor

@i53577 i53577 commented Jan 9, 2024

No description provided.

scarai-sap and others added 30 commits September 18, 2023 16:21
As a customer, I want to view and update my preferred notification channels
format the code and other changes from code review comments
add cms config
@developpeurweb
Copy link
Contributor

@developpeurweb , Please review it again

@scarai-sap it will be @morganm58 taking over the CSS review, as he is the Spartacus Theme Owner.

.myaccount-password-label {
@include type();
min-width: 7.5rem;
padding: 0 1.2rem 0 0;
Copy link
Contributor

@morganm58 morganm58 Feb 7, 2024

Choose a reason for hiding this comment

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

Substitute padding-left and padding-right for padding-inline-start and padding-inline-start. We need these for directionality to work.
padding-top: 0
padding-inline-end: 1.2rem
padding-bottom: 0
padding-inline-start: 0


form {
label {
padding-bottom: 0.75rem !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid using !important. Can you make the scss more specific so you won't need to use !important.

}

.consent-details {
margin: 0 0 0 2.815rem;
Copy link
Contributor

Choose a reason for hiding this comment

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

Substitute margin-left and margin-right for margin-inline-start and margin-inline-start. We need these for directionality to work.
padding-top: 0
padding-inline-end: 0
padding-bottom: 0
padding-inline-start: 2.815rem

}

.custom-switch {
margin: 0 0 0 -2.25rem;
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment as above


%cx-my-account-v2-notification-preference {
.np-content-center {
-ms-flex-pack: center !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid using !important.

%cx-my-account-v2-notification-preference {
.np-content-center {
-ms-flex-pack: center !important;
justify-content: center !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid using !important for all instances.

Copy link
Contributor

@morganm58 morganm58 left a comment

Choose a reason for hiding this comment

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

I added comments to a few of the scss files.

@github-actions github-actions bot marked this pull request as draft February 8, 2024 02:40
@scarai-sap
Copy link
Contributor

@morganm58 , all comments fixed

@scarai-sap scarai-sap marked this pull request as ready for review February 8, 2024 02:53
@github-actions github-actions bot marked this pull request as draft February 8, 2024 03:17
@scarai-sap scarai-sap marked this pull request as ready for review February 8, 2024 06:22
@github-actions github-actions bot marked this pull request as draft February 9, 2024 03:23
@scarai-sap scarai-sap marked this pull request as ready for review February 9, 2024 03:29
Copy link
Contributor

@developpeurweb developpeurweb left a comment

Choose a reason for hiding this comment

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

@Platonn LGTM.

@Platonn Platonn merged commit 6c4271c into develop Feb 12, 2024
26 checks passed
@Platonn Platonn deleted the feature/my-account-v2-holidays branch February 12, 2024 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.