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

Get credential phase 2 #1575

Merged
merged 55 commits into from
Jul 10, 2024
Merged

Get credential phase 2 #1575

merged 55 commits into from
Jul 10, 2024

Conversation

BaskarMitrah
Copy link
Collaborator

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@BaskarMitrah BaskarMitrah requested a review from timkim May 8, 2024 15:49
@BaskarMitrah BaskarMitrah marked this pull request as draft May 8, 2024 16:04
@timkim timkim marked this pull request as ready for review May 20, 2024 17:45
@timkim timkim marked this pull request as draft May 20, 2024 17:47
Copy link
Collaborator

@deepessh deepessh left a comment

Choose a reason for hiding this comment

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

Thank you so much @BaskarMitrah!

deepessh and others added 19 commits June 6, 2024 15:28
* -- Removed the Subscription Error

* renamed NestedAlertContent files

* --Fixed Sample code download issues

* --fixed the project link to the card after create the credential

* --Fixed the checkmark issues on the picker in the return flow project details

* Fixed scopes details as dynamic

* --Separated the Service and formfields page

---------

Co-authored-by: BaskarMitrah <mit44186@adobe.com>
Co-authored-by: Manimaran Kottursamy <mit80447@adobe.com>
* Fix : Error in form details clears the form. Should not clear it.
The toast message does not dismiss on it's own

* --Added validation for the domain

* --Fixed design issues

* --Credential name validation
* --Fixed the issues on the generate token & client secret

* fix : added copy icon for access token & client secret
… up automatically, I have to refresh the page or the request access button keeps showing. (#1596)

If I fill out the form and change the org, the form fields reset. Only the dev terms acceptance should reset. Rest everything should stay the same.

Co-authored-by: BaskarMitrah <mit44186@adobe.com>
fix: Personal Developer Org bug on org switcher and error states (#1598)
@deepessh deepessh marked this pull request as ready for review July 9, 2024 16:15
import { PreviousProject } from './PreviousProject';
import { Organization } from './Organization';
import { CredentialForm } from './CredentialForm';
import { RetunrSideComp } from './Return/RetunrSideComp';
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo

Copy link
Collaborator

Choose a reason for hiding this comment

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

On line 9.

"packageManager": "yarn@3.2.3"
"packageManager": "yarn@3.2.3",
"dependencies": {
"@adobe/react-spectrum": "^3.35.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure about including react-spectrum as a dependency. It looks like we are just using it for a few ui components. I would like to not have react-spectrum as it would increase the bloat/size of the theme for all who consume it. Moreover, most people who do use the theme won't be using the create credentials component.

I think moving forward we would want to remove whatever ui components we are getting from it and just create our own version of those ui components.

Copy link
Collaborator

@timkim timkim left a comment

Choose a reason for hiding this comment

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

Only one change needed as noted by the typo I found.

@timkim timkim merged commit 2200844 into main Jul 10, 2024
3 checks passed
@timkim timkim deleted the get_credential_phase2 branch July 10, 2024 23:37
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