-
Notifications
You must be signed in to change notification settings - Fork 156
Created skeleton transaction web view #1507
Conversation
enzyme/tests/Page.test.tsx
Outdated
@@ -0,0 +1,28 @@ | |||
import React, { Component } from 'react'; |
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.
As @cazfletch mentioned, we should probably make Page
and Sidebar
less generic as they're unlikely to be imported/used by other components.
enzyme/tests/Page.test.tsx
Outdated
expect(component).toMatchSnapshot(); | ||
}); | ||
|
||
afterEach(async () => { |
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.
Could we move afterEach
below beforeEach
but before it
as we do it similarly in our extension tests.
enzyme/tests/Sidebar.test.tsx
Outdated
import sinonChai from 'sinon-chai'; | ||
chai.should(); | ||
chai.use(sinonChai); | ||
const should: Chai.Should = chai.should(); |
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.
You can remove this line as we do chai.should()
above.
src/App.tsx
Outdated
@@ -34,8 +33,7 @@ class App extends React.Component<any> { | |||
<Router> | |||
<div> | |||
<Route render={() => <Redirect push to={this.state.redirectPath}/>}></Route> | |||
<Route path='/one' component={OneComponent}></Route> | |||
<Route path='/two' component={TwoComponent}></Route> | |||
<Route path='/one' render={() => <Page activeSmartContract="penguinContract@0.0.1"/>}></Route> |
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.
Probably worth changing the path /one
to /transaction
or something.
If OneComponent
isn't being used anymore we can probably delete it.
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.
Same with TwoComponent
!
src/components/Page/Page.scss
Outdated
@import 'node_modules/carbon-components/scss/components/accordion/_accordion'; | ||
@import 'node_modules/carbon-components/scss/components/link/link'; | ||
|
||
body { |
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.
Is this styling something we should be doing at the App level instead?
src/components/Page/Page.scss
Outdated
.page-contents { | ||
display: flex; | ||
flex-direction: column; | ||
margin-left: 325px; |
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.
For all page layout, we should probably try to use the carbon grid
src/components/Page/Page.tsx
Outdated
<div className="contents-right" id="contents-right"> | ||
<h5>Getting Started</h5> | ||
<Accordion> | ||
<AccordionItem title="Create a new transaction"></AccordionItem> |
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.
When changing the theme from dark to light, this text doesn't seem to change colour.
97e4483
to
044a5f6
Compare
f45dd4c
to
dbe8a51
Compare
0b50a52
to
25c3639
Compare
Yesterday Caroline asked me to get the build to publish the results of the jest tests like we do for the unit tests. I think I have the test report publishing, but am not sure how to tackle publishing the coverage report. Will have another go on Monday 👍 |
package.json
Outdated
@@ -734,6 +734,14 @@ | |||
"command": "gatewaysExplorer.testSmartContractEntry", | |||
"when": "view == gatewaysExplorer && viewItem == blockchain-instantiated-multi-contract-item || viewItem == blockchain-instantiated-contract-item || viewItem == blockchain-contract-item || viewItem == blockchain-instantiated-unknown-item" | |||
}, | |||
{ | |||
"command": "reactPage.open", |
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 be worth change reactPage.open
to transactionPage.open
or something similar.
|
||
if (this.state.panelType === 'buttons') { | ||
panelTSX = ( | ||
<div className="panel-container" id="panel-container"> |
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.
Spotted it in a few places - I don't think you want an id & a class. I think you could probably remove these ids.
<div className="inner-container bx--row"> | ||
<Sidebar/> | ||
<div className="page-contents bx--col" id="page-contents"> | ||
<div className="titles-container" id="titles-container"> |
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.
Not sure you need these ids as well!
437dcd1
to
8eed3ac
Compare
Can be accessed by right-clicking on a smart contract in the gateways view Signed-off-by: Erin Hughes <Erin.Hughes@ibm.com>
8eed3ac
to
61a6d99
Compare
Closes #1475 and contributes to #1155
Signed-off-by: Erin Hughes Erin.Hughes@ibm.com