-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add components #12
Add components #12
Changes from 19 commits
3bf080b
5565c6c
b068deb
9259cd6
4e6cc32
7be2f0f
fbfbe57
aff72a4
e02bb16
3099834
6b14aa2
dc89b0f
41906f5
86cd6a0
f8425ca
463c0e8
505cab6
2bec60e
0489428
100039a
ff11f77
1680315
eab56f6
2734698
80980ba
0d64384
878f097
4244c49
86a5528
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
0 silly argv { _: [ 'run' ], | ||
0 silly argv scope: '@apicurio/studio', | ||
0 silly argv lernaVersion: '3.20.2', | ||
0 silly argv '$0': | ||
0 silly argv '/Users/cmolloy/Documents/repos/apicurio-studio-react-poc/node_modules/.bin/lerna', | ||
0 silly argv script: 'start' } | ||
1 notice cli v3.20.2 | ||
2 verbose rootPath /Users/cmolloy/Documents/repos/apicurio-studio-react-poc | ||
3 notice filter including "@apicurio/studio" | ||
4 info filter [ '@apicurio/studio' ] | ||
5 info Executing command in 1 package: "yarn run start" | ||
6 silly npmRunScript start [] @apicurio/studio | ||
7 silly getExecOpts /Users/cmolloy/Documents/repos/apicurio-studio-react-poc/packages/studio undefined | ||
8 error MaxBufferError: stderr maxBuffer exceeded | ||
8 error at PassThrough.stream.on (/Users/cmolloy/Documents/repos/apicurio-studio-react-poc/node_modules/get-stream/index.js:41:19) | ||
8 error at PassThrough.emit (events.js:187:15) | ||
8 error at addChunk (_stream_readable.js:283:12) | ||
8 error at readableAddChunk (_stream_readable.js:260:13) | ||
8 error at PassThrough.Readable.push (_stream_readable.js:219:10) | ||
8 error at PassThrough.Transform.push (_stream_transform.js:151:32) | ||
8 error at PassThrough.afterTransform (_stream_transform.js:92:10) | ||
8 error at PassThrough._transform (_stream_passthrough.js:42:3) | ||
8 error at PassThrough.Transform._read (_stream_transform.js:190:10) |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
{ | ||
"apis": [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Create a bug to move this out of studio into a package called mock under packages. We can then include it in as a package under devDependencies. This way we won't add unnecessary mock data files to the studio package. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Created this issue #15 |
||
{ | ||
"id": 0, | ||
"name": "Pet Store API", | ||
"description": "A sample API that uses a pet store as an example to demonstrate features in the OpenAPI 3.0 specification.", | ||
"type": "Open API 3.0.2", | ||
"template": "Blank API", | ||
"date": "January 29, 2020", | ||
"author": "[email protected]" | ||
}, | ||
{ | ||
"id": 1, | ||
"name": "Sports Store API", | ||
"description": "A sample API that uses a pet store as an example to demonstrate features in the OpenAPI 3.0 specification.", | ||
"type": "Open API 3.0.2", | ||
"template": "Blank API", | ||
"date": "January 29, 2020", | ||
"author": "[email protected]" | ||
}, | ||
{ | ||
"id": 2, | ||
"name": "Clothes Store API", | ||
"description": "A sample API that uses a pet store as an example to demonstrate features in the OpenAPI 3.0 specification.", | ||
"type": "Open API 3.0.2", | ||
"template": "Blank API", | ||
"date": "January 29, 2020", | ||
"author": "[email protected]" | ||
}, | ||
{ | ||
"id": 3, | ||
"name": "Weather API", | ||
"description": "A sample API that uses a pet store as an example to demonstrate features in the OpenAPI 3.0 specification.", | ||
"type": "Open API 3.0.2", | ||
"template": "Blank API", | ||
"date": "January 29, 2020", | ||
"author": "[email protected]" | ||
} | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,57 +1,28 @@ | ||
import React, { Component, ReactNode } from "react"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can change this to:
|
||
import { | ||
Page, | ||
PageSectionVariants, | ||
PageSection | ||
} from "@patternfly/react-core"; | ||
import { Button, Level, LevelItem, Page, PageSectionVariants, PageSection, Title } from "@patternfly/react-core"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Button, level, levelitem, pagesectionvariant, pagesection, and title are no longer being used can you remove them. |
||
import AppHeader from "./appHeader"; | ||
import AppSidebar from "./appSidebar"; | ||
import {BrowserRouter as Router, Route} from 'react-router-dom'; | ||
import * as Pages from './pages'; | ||
export default class App extends Component { | ||
public state = { | ||
activeMenuGroup: "", | ||
activeMenuGroupItem: "" | ||
}; | ||
import './app.css'; | ||
|
||
public render() { | ||
const { activeMenuGroup, activeMenuGroupItem } = this.state; | ||
type AppProps = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we aren't passing in specific props you can delete this. |
||
} | ||
|
||
const section = ( | ||
<PageSection variant={PageSectionVariants.light}> | ||
<Route path='/' exact={true} component={Pages.Dashboard}/> | ||
<Route path='/dashboard' exact={true} component={Pages.Dashboard}/> | ||
<Route path='/apis' exact={true} component={Pages.ViewApis}/> | ||
<Route path='/apis/create' exact={true} component={Pages.CreateAPI}/> | ||
<Route path='/apis/import' exact={true} component={Pages.ImportAPI}/> | ||
<Route path='/settings/profile' exact={true} component={Pages.UserProfile}/> | ||
<Route path='/settings/accounts' exact={true} component={Pages.LinkedAccounts}/> | ||
<Route path='/settings/validation' exact={true} component={Pages.Validations}/> | ||
</PageSection> | ||
); | ||
export default class App extends Component<AppProps> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Change this to a function component.
|
||
constructor(props: AppProps) { | ||
super(props); | ||
} | ||
render() { | ||
return ( | ||
<Router> | ||
<Page | ||
isManagedSidebar={true} | ||
header={<AppHeader />} | ||
sidebar={ | ||
<AppSidebar | ||
activeMenuGroup={activeMenuGroup} | ||
activeMenuGroupItem={activeMenuGroupItem} | ||
onSelect={this.onNavSelect} | ||
/> | ||
} | ||
<Page isManagedSidebar={true} header={<AppHeader />} | ||
> | ||
{section} | ||
<Route path='/' exact={true} component={Pages.Dashboard}/> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should open a Jira issue to have Lucia design a route for page not found. We need an elegant way to handle 404. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
<Route path='/dashboard' exact={true} component={Pages.Dashboard}/> | ||
<Route path='/create-api' exact={true} component={Pages.CreateApi}/> | ||
<Route path='/import-api' exact={true} component={Pages.ImportApi}/> | ||
</Page> | ||
</Router> | ||
); | ||
} | ||
|
||
private onNavSelect = ({ groupId, itemId }: any) => { | ||
this.setState({ | ||
activeMenuGroup: groupId, | ||
activeMenuGroupItem: itemId | ||
}); | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
import React, { ReactNode } from 'react'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Change this too:
ReactNode isn't needed anymore. |
||
import { Card, CardHead, CardHeader, CardBody, CardFooter, CardActions } from '@patternfly/react-core'; | ||
import {AppTag} from './appTag'; | ||
import ApicurioIcon from './assets/apicurio-icon.png'; | ||
import './app.css' | ||
|
||
type AppCardProps = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Best practices for typescript now has changed to use interfaces instead of types here. Can you change this to an interface: You should also disable the rule in tslint.json to start with I for interfaces. React community doesn't usually follow that anymore. To do this add the following rule to the tslint.json file:
|
||
apiName: string, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't believe all these props are required. We should change these to match the props in the model so it will map 1 to 1 when retrieve this from the server.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep updating these now that I know which models we're using! |
||
apiDescription: string, | ||
apiTag1: string, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be an array of tags. They shouldn't be required I believe.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, updated! |
||
apiTag2: string | ||
} | ||
|
||
class AppCard extends React.Component<AppCardProps> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Convert this to a function component since state isn't being used.
|
||
constructor(props: AppCardProps) { | ||
super(props); | ||
this.state = { | ||
check: false | ||
}; | ||
} | ||
render() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should spread the props in the render function
|
||
return ( | ||
<Card> | ||
<CardHead> | ||
<img src={ApicurioIcon}/> | ||
<CardActions> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Change |
||
</CardActions> | ||
</CardHead> | ||
<CardHeader className="app-card-view-card-header"> | ||
{this.props.apiName} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. {name} |
||
</CardHeader> | ||
<CardBody className="app-card-view-card-body"> | ||
{this.props.apiDescription} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. {description} |
||
</CardBody> | ||
<CardFooter> | ||
<div className="app-api-tag-group"> | ||
<AppTag | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use a map function here to create the AppTag.
|
||
text={this.props.apiTag1} | ||
/> | ||
<AppTag | ||
text={this.props.apiTag2} | ||
/> | ||
</div> | ||
</CardFooter> | ||
</Card> | ||
); | ||
} | ||
} | ||
|
||
export default AppCard; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import React, { ReactNode } from 'react'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove { ReactNode } |
||
import { Gallery, GalleryItem } from '@patternfly/react-core'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove GalleryItem |
||
import AppCard from './appCard'; | ||
import data from '../api-data.json'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Create a bug to move this to a package called mock under packages. We can update our code so we only pull in the mock package as a dependency for development. You can assign it to this sprint in Jira. |
||
|
||
export const AppCardView: React.FunctionComponent<any> = (props) => { | ||
const apiData = data.apis; | ||
|
||
const cardList = apiData.map((api) => | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update to the following:
While this is good for now I think when we get the backend communication setup this will change to a prop where you pass in array of AppCards. While find out. :-) |
||
<AppCard | ||
apiName={api.name} | ||
apiDescription={api.description} | ||
apiTag1="Tag 1" | ||
apiTag2="Another tag" | ||
/> | ||
); | ||
|
||
return ( | ||
<Gallery gutter="md"> | ||
{cardList} | ||
</Gallery> | ||
); | ||
} | ||
|
||
export default AppCardView; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
import React, { ReactNode } from 'react'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Delete {ReactNode} |
||
import {DataList} from '@patternfly/react-core'; | ||
import AppDataListItem from './appDataListItem'; | ||
import data from '../api-data.json'; | ||
|
||
export interface AppDataListProps { | ||
viewDetails: (ev: React.MouseEvent<HTMLButtonElement>) => void, | ||
selectItem: (ev: React.MouseEvent<HTMLButtonElement>) => void | ||
} | ||
|
||
export interface AppDataListState { | ||
selectedDataListItemId: string | ||
} | ||
|
||
const apiData = data.apis; | ||
|
||
class AppDataList extends React.Component<AppDataListProps, AppDataListState> { | ||
constructor(props: AppDataListProps) { | ||
super(props); | ||
this.state = { | ||
selectedDataListItemId: '' | ||
}; | ||
} | ||
|
||
onSelectDataListItem = (id: string) => { | ||
this.setState({ selectedDataListItemId: id }); | ||
this.props.selectItem(id); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. id is not assignable to React.MouseEvent. It's a type mismatch since it's a string. |
||
this.props.keyListItem(id); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. keyListItem is missing from AppDataListProps. |
||
console.log('is there a key here' + key); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove the console.log. |
||
} | ||
|
||
render() { | ||
const listItems = apiData.map((api, index) => | ||
<AppDataListItem | ||
apiName={api.name} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. need to add key={index} to AppDataListItem |
||
apiDescription={api.description} | ||
apiTag1="Tag" | ||
apiTag2="Another tag" | ||
rowid={index} | ||
id={api.id} | ||
onClick={this.props.viewDetails} | ||
/> | ||
); | ||
return ( | ||
<DataList | ||
aria-label="selectable data list" | ||
selectedDataListItemId={this.state.selectedDataListItemId} | ||
onSelectDataListItem={this.onSelectDataListItem} | ||
> | ||
{listItems} | ||
</DataList> | ||
); | ||
} | ||
} | ||
|
||
export default AppDataList; |
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 don't know much about lerna - but this sounds like a file that shouldn't be added to git.
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.
@christiemolloy Can you remove this log file. Should update the .gitignore to not check in .log files as well.
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 and updated gitignore