-
Notifications
You must be signed in to change notification settings - Fork 387
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: CXSPA-4442 New components for email and personal info #17839
Conversation
Be frankly, I don't know why we need it. I guess you are right. We need add it as other component does |
feature-libs/user/profile/components/new-user-profile/new-profile.component.spec.ts
Outdated
Show resolved
Hide resolved
please add accessibility test |
let me check whether it's in the scope |
|
Your e2e tests are already here. So I am curious why accessibility test is not. But it's ok for me to use another ticket that you mentioned to track it. |
@@ -20,7 +20,7 @@ export function registerAndLogin() { | |||
export function testUpdateEmailAndLogin() { | |||
it('should update his email address and login', () => { | |||
const newUid = generateMail(randomString(), true); | |||
cy.get('cx-update-email').within(() => { | |||
cy.get('cx-update-email, cx-my-new-account-v2-email').within(() => { |
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.
if v1 and v2 will exist at the same time, then does this change mean v1's tests will not pass in the future?
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.
this grammar means A or B in here.
@@ -23,12 +23,19 @@ export function updateProfile(user?: SampleUser) { | |||
); | |||
cy.get('[formcontrolname="lastName"]').should('have.value', user.lastName); | |||
} | |||
cy.get('.myaccount-enhancedUI-editButton') |
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.
maybe it's better to add new functions instead of modifying existing ones that for v1.
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.
two sides of the coin, modify/reuse code make it easily if we wanna change sth in the future
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 don't get the point. Does V1 also have this element myaccount-enhancedUI-editButton
? Won't it cause any failure in V1‘s tests?
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.
if no this element, not click edit. ===> in V1, it loaded with edit model
if with this element. click edit ==> n V2, transfer to edit model.
Then we can reuse edit feature as it's totally same.
ATT. Below is how it works now
Update new video with new css on 18:12 21 Sept. Please check it again
2023-09-21_18-12-04.mp4