-
Notifications
You must be signed in to change notification settings - Fork 60
Fix header component - Closes #506 #542
Changes from all commits
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 |
---|---|---|
|
@@ -16,3 +16,7 @@ | |
.material-icons{ | ||
font-size: 24px !important; | ||
} | ||
|
||
.menu { | ||
right: -16px !important; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import React from 'react'; | ||
import { connect } from 'react-redux'; | ||
|
||
export const PrivateWrapperComponent = ({ isAuthenticated, children }) => ( | ||
isAuthenticated && <span>{ children }</ span> | ||
); | ||
|
||
const mapStateToProps = state => ({ | ||
isAuthenticated: !!state.account.publicKey, | ||
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 something else then 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. currently, all private routes look to the same params. 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. In private routes, it makes no user-visible difference. But on the login page if user is on slower network, it can longer time for the request and only then user is redirected from login page. Logout and Send buttons should not be on login page in the meantime. 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 was fixed in the meantime by some other changes. |
||
}); | ||
|
||
export default connect(mapStateToProps)(PrivateWrapperComponent); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import React from 'react'; | ||
import { expect } from 'chai'; | ||
import { shallow } from 'enzyme'; | ||
import { PrivateWrapperComponent } from '.'; | ||
|
||
const Private = () => <h1>Private</h1>; | ||
|
||
describe('PrivateWrapperComponent', () => { | ||
const isAuth = isAuthenticated => ( | ||
shallow( | ||
<PrivateWrapperComponent isAuthenticated={isAuthenticated}> | ||
<Private/ > | ||
</PrivateWrapperComponent>, | ||
) | ||
); | ||
it('should render children components if user is authenticated', () => { | ||
const wrapper = isAuth(true); | ||
expect(wrapper.find(Private)).to.have.length(1); | ||
}); | ||
|
||
it('should do not render children components if user is not authenticated', () => { | ||
const wrapper = isAuth(false); | ||
expect(wrapper.find(Private)).to.have.length(0); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,7 +39,7 @@ class Transactions extends React.Component { | |
length: parseInt(res.count, 10), | ||
}); | ||
}) | ||
.catch(error => console.error(error.message)); | ||
.catch(error => console.error(error.message)); //eslint-disable-line | ||
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 see that finally somebody got annoyed enough by the warning in console :-) Though I would prefer to disable the specific rule. In this case a believe it is |
||
} | ||
} | ||
|
||
|
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.
This could be less (maybe
right: -8px !important;
) and the put the rest on.wrapper
..wrapper
has too much space on the right compared to nano 1.0.0 and not enough at the bottom.I mean something like this is missing
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.
menu
is a theme param, and it passes to the component and merges with other styles. it is so hard to styleMenu
component throughIconMenu
component.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 mean
.wrapper
which is defined and used in ourheader
component. Nothing extra inIconMenu
https://github.com/alepop/lisk-nano/blob/2c484cfd1565634b4f682fbf207ee2bc3ce3bb42/src/components/header/header.css#L1-L4
https://github.com/alepop/lisk-nano/blob/2c484cfd1565634b4f682fbf207ee2bc3ce3bb42/src/components/header/headerElement.js#L12-L13
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.
This is still an issue
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.
Do i need to fix it in other pr?
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, please.