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

19332 - Short name account link modal #2720

Merged
merged 18 commits into from
Feb 13, 2024

Conversation

rodrigo-barraza
Copy link
Collaborator

Issue #:
https://github.com/bcgov/entity/issues/

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the sbc-auth license (Apache 2.0).

@rodrigo-barraza rodrigo-barraza marked this pull request as ready for review February 9, 2024 17:02
@rodrigo-barraza rodrigo-barraza changed the title Short name account linking 19332 - Short name account link modal Feb 9, 2024
Copy link
Collaborator

@seeker25 seeker25 left a comment

Choose a reason for hiding this comment

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

see above

@seeker25
Copy link
Collaborator

seeker25 commented Feb 9, 2024

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://bcregistry-account-dev--pr-2720-mgg2g9o6.web.app

@seeker25
Copy link
Collaborator

seeker25 commented Feb 9, 2024

Modal size needs to be increased, should be 720x, I think it's 620 ish

@seeker25
Copy link
Collaborator

seeker25 commented Feb 9, 2024

image

@seeker25
Copy link
Collaborator

seeker25 commented Feb 9, 2024

button alignment and styling needs a tweak:

image

@seeker25
Copy link
Collaborator

seeker25 commented Feb 9, 2024

Close -> Cancel it should be outlined in style

@seeker25
Copy link
Collaborator

seeker25 commented Feb 9, 2024

spacing here:
image

@seeker25
Copy link
Collaborator

seeker25 commented Feb 9, 2024

image

@seeker25
Copy link
Collaborator

seeker25 commented Feb 9, 2024

I tried to link,
image

@seeker25
Copy link
Collaborator

seeker25 commented Feb 9, 2024

I tried again, pressed it, it switched tabs but didnt' close the dialog:

image

@seeker25
Copy link
Collaborator

seeker25 commented Feb 9, 2024

Also think you need to handle "EFT_SHORT_NAME_ALREADY_MAPPED" error message

@ochiu
Copy link
Collaborator

ochiu commented Feb 12, 2024

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://bcregistry-account-dev--pr-2720-mgg2g9o6.web.app

@ochiu
Copy link
Collaborator

ochiu commented Feb 12, 2024

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://bcregistry-account-dev--pr-2720-mgg2g9o6.web.app

@seeker25
Copy link
Collaborator

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://bcregistry-account-dev--pr-2720-mgg2g9o6.web.app

Copy link
Collaborator

@seeker25 seeker25 left a comment

Choose a reason for hiding this comment

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

Could you update package + package-lock.json again please?

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
4.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@seeker25 seeker25 merged commit 048a15b into bcgov:main Feb 13, 2024
4 of 6 checks passed
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.

4 participants