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

Add components #12

Merged
merged 29 commits into from
Feb 5, 2020
Merged

Add components #12

merged 29 commits into from
Feb 5, 2020

Conversation

christiemolloy
Copy link
Contributor

@christiemolloy christiemolloy commented Jan 23, 2020

Add components to the UI:

  • Dashboard (emptyState, CardView, ListView, Drawer)
  • CreateAPI: Form
  • ImportAPI: Form

lerna-debug.log Outdated
@@ -0,0 +1,22 @@
0 silly argv { _: [ 'run' ],
Copy link
Member

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.

Copy link
Collaborator

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed and updated gitignore

@EricWittmann
Copy link
Member

Are *.tsx files just typescript files? Have I been naming my typescript files incorrectly all this time? :)

@christiemolloy christiemolloy requested a review from dlabaj January 23, 2020 20:17
@EricWittmann EricWittmann changed the title Add components [WIP] Add components Jan 28, 2020
@EricWittmann
Copy link
Member

I fixed up the Keycloak problems. You can see that in this PR: #14

So you should be able to restore the keycloak stuff in this PR.

Copy link
Collaborator

@dlabaj dlabaj left a comment

Choose a reason for hiding this comment

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

Let me know if you want to meet to go over the changes.

lerna-debug.log Outdated
@@ -0,0 +1,22 @@
0 silly argv { _: [ 'run' ],
Copy link
Collaborator

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.

@@ -16,4 +16,84 @@
.pf-c-page {
width: 100%;
height: 100vh;
}

.pf-c-page__main-section {
Copy link
Collaborator

Choose a reason for hiding this comment

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

.pf-c stuff are also found in patternfly right? Instead of overriding the classes should we create new classes and set set these modifications in those classes? For example create a appicurio-page_main-section class with these settings. We did that in integreatly which seemed like a good approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think thats a great idea thanks for pointing that out!

>
{section}
<Route path='/' exact={true} component={Pages.Dashboard}/>
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

import './app.css'

type AppCardProps = {
apiName: string,
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

interface AppCardProps { name: string, description: string, tags?: string[] }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep updating these now that I know which models we're using!

@@ -0,0 +1,40 @@
{
"apis": [
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created this issue #15

</Tabs>
<div>
<TabContent eventKey={0} id="refTab1Section" ref={this.contentRef1} aria-label="Tab item 1">
{this.props.date}
Copy link
Collaborator

Choose a reason for hiding this comment

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

date and author are not defined in the AppTabsProps.

};

render() {
return (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you update this to destructure the props object.

import { Breadcrumb, BreadcrumbItem, Button, Form, FormGroup, TextInput, TextArea, FormSelectOption, FormSelect, Checkbox, ActionGroup, Radio, Title, PageSection, PageSectionVariants } from '@patternfly/react-core';
import '../../app.css'

type CreateApiProps = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can remove this.

type CreateApiProps = {
}

type CreateApiState = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

change to an interface.

@@ -3,27 +3,27 @@ import ReactDOM from "react-dom";
import "@patternfly/react-core/dist/styles/base.css";
import App from './app/app';
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can undo these change and rebase to get the latest fix for keycloak.

dlabaj
dlabaj previously approved these changes Feb 4, 2020
Copy link
Collaborator

@dlabaj dlabaj left a comment

Choose a reason for hiding this comment

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

LGTM

@EricWittmann
Copy link
Member

This still says "WIP" so just let me know when you want it merged.

@EricWittmann
Copy link
Member

Need to resolve conflicts first. :)

@EricWittmann EricWittmann changed the title [WIP] Add components Add components Feb 5, 2020
@EricWittmann EricWittmann merged commit be1cb65 into Apicurio:master Feb 5, 2020
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.

3 participants