-
Notifications
You must be signed in to change notification settings - Fork 2
Use sso/login as link to share access when user is logged in with SSO
#1883
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
base: main
Are you sure you want to change the base?
Conversation
Need to wrap in a span as the copy button does not accept a ref
sso/login as link to share access when user is logged in with SSO
| color={isCopied ? 'success' : undefined} | ||
| onClick={() => handleCopy(writeValue)} | ||
| onClick={(event) => { | ||
| event.stopPropagation(); |
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.
Prevent the copy link button from selecting the row when clicked
| id: 'cta.inviteLink', | ||
| })} | ||
| </CopyToClipboardButton> | ||
| <span> |
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.
Handles an issue with passing ref
| const LOGIN_LINK_URL = `${window.location.origin}${unauthenticatedRoutes.login.path}`; | ||
| const SSO_LOGIN_LINK_URL = `${window.location.origin}${unauthenticatedRoutes.sso.login.fullPath}`; |
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.
Was initially wondering if we could do some cool magic and mess with the routes dynamically... but this is much easier and low scope approach.
Issues
#1773
Changes
1773
Tests
Manually tested
Automated tests
Playwright tests ran locally
Screenshots
Share link is now
sso/loginifSSOwas usedShare link still
/loginifOAuthwas used