-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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: remove auth section #2022
Conversation
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.
Should we deprecate noAutoAuth
? What do you think? @anastasiia-developer @RomanHotsiy @Oprysk
{ openapi: '3.0', info: { title: 'test', version: '0' }, paths: {} }, | ||
undefined, | ||
options, | ||
const spec: OpenAPISpec = { |
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 better move spec to fixtures
?
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.
Does it cover all security types, which we support?
oauth2: 'OAuth2',
apiKey: 'API Key',
http: 'HTTP',
openIdConnect: 'OpenID Connect',
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 better move spec to fixtures?
Done 😄
…nto feat/remove_auth_section
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.
LGTM! Good work 🎉
Are these screenshots updated? I see too much vertical gap between |
RequiredScopes?: JSX.Element; | ||
} | ||
|
||
export class OAuthFlow extends React.PureComponent<OAuthFlowProps> { |
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.
It would be great to use functional component here
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.
done
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.
@anastasiia-developer Previously it was a PureComponent
after refactoring it to the function component we need to keep the performance in the rendering perspective. So we might use React memo
HOC here, what do you think?
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.
yes, we might
I'm going to add that
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.
Done
What/Why/How?
feat: remove auth section
Reference
Testing
Screenshots (optional)
Security unexpanded
Security expanded
See more
See less
Check yourself