-
Notifications
You must be signed in to change notification settings - Fork 81
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
Fix missing versions in Aboutt Modal #254
Conversation
6495ba9
to
4815fe5
Compare
console.log('componentDidMount'); | ||
ApplicationInfoAPI.get('') | ||
.then(result => { | ||
console.log(result); |
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.
Might want to remove this
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.
Removed all console.logs
applicationInfo: { server_version: string; pulp_ansible_version: string }; | ||
} | ||
|
||
export class AboutModalWindow extends React.Component<IProps, IState> { |
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'm a little conflicted about classifying this as a component instead of a container. On one hand, it's making API requests and it has zero re-usability. On the other hand It's not a standalone page like all of our other containers are.
My gut reaction is to move this into containers. 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.
Moved :)
4815fe5
to
107d29e
Compare
Move AboutModal to be a component Fix-Issue: AAH-161
107d29e
to
9c8559c
Compare
Move AboutModal to be a component to get componentDidMount to work properly.
Fix-Issue: AAH-161
Steps to reproduce:
Before:

After:
