Ida/implement start page #324
Conversation
idali0226
force-pushed
the
ida/implement-start-page
branch
4 times, most recently
from
6611a08
to
0275da5
Compare
| @@ -0,0 +1,10 @@ | |||
| module.exports = function capitalizeFirstLetterOfEachWord(string) { | |||
There was a problem hiding this comment.
it's great you add utilities!
There was a problem hiding this comment.
even if you don't need it for username when you use name instead, we can still keep this utility
| return string | ||
| } | ||
| return string | ||
| .toLowerCase() |
There was a problem hiding this comment.
maybe we don't have to make everything lowercase. it might be some upper case inside a word that should be preserved, e.g. "GitHub"
| return string | ||
| .toLowerCase() | ||
| .split(' ') | ||
| .map(a => a.charAt(0).toUpperCase() + a.substr(1).toLowerCase()) |
There was a problem hiding this comment.
| module: 'start', | ||
| textKey: 'welcome', | ||
| })} ${capitalizeFirstLetterOfEachWord( | ||
| username |
There was a problem hiding this comment.
instead of username you could use name which we also should have:
|
|
||
| class Start extends Component { | ||
| render() { | ||
| const { i18n: { moduleTranslate }, username } = this.props | ||
|
|
||
| return ( | ||
| <div> |
There was a problem hiding this comment.
use React.Fragment instead
| </Segment> | ||
| </Grid.Column> | ||
| </Grid.Row> | ||
| <div className="ui center aligned grid"> |
There was a problem hiding this comment.
don't use classNames like this, use the props on Grid instead. textAlign="center".
and maybe this div is not needed
| <div className="ui center aligned grid"> | ||
| <Grid | ||
| columns={2} | ||
| style={{ marginTop: '6.25em', maxWidth: '60em' }} |
There was a problem hiding this comment.
if you want a maxWidth like this it's better to add the prop container that achieves the same thing.
You should look at those:
https://react.semantic-ui.com/collections/grid/#responsive-variations-container
https://react.semantic-ui.com/elements/container/
In general it would be good if you look more into the Semantic docs so you use the built-in features when possible
| style={{ | ||
| color: '#000000a0', | ||
| fontWeight: 200, | ||
| lineHeight: '2.5rem', |
There was a problem hiding this comment.
I guess we can leave this for now, but it's questionable if it has to be this custom styling. This is not reusable, to do so you would have to add it as a custom style for header. But we are short on time and it would make sense to use one of the existing header variations. but we can leave it as it is.
| </Button> | ||
| </NavLink> | ||
| <NavLink to="/app/specimens/mammals"> | ||
| <Button basic color="blue" size="huge" type="button"> |
There was a problem hiding this comment.
instead of basic color="blue" I think you should use basic primary, at least I guess this color should change if we change the color of primary buttons
| textKey: 'needHelp', | ||
| })}{' '} | ||
| <a href="mailto:team@mail.dina-web.net"> | ||
| dina-collections@nrm.se |
There was a problem hiding this comment.
use the actual email address: team@mail.dina-web.net
what's in the sketch is just an example
There was a problem hiding this comment.
It is a generic contact address setup for dina related issues.
| module: 'start', | ||
| textKey: 'currentlyVersion', | ||
| })} | ||
| 1.0 |
There was a problem hiding this comment.
we're not on version 1.0. if you look in package.json in the root we're "0.5.2" now. what is in the sketch is not always meant literally, here it means to show the current version, not hardcode 1.0 :-)
you will have to get the version from package.json somehow... and that is not trivial, because create-react-app blocks import of anything outside /src unless it's the ui package.json or something symlinked. so we probably need to add a function in common that imports the root package.json and returns the version. maybe I need to help with that
some info in here: facebook/create-react-app#2468
https://stackoverflow.com/questions/44114436/the-create-react-app-imports-restriction-outside-of-src-directory
idali0226
force-pushed
the
ida/implement-start-page
branch
2 times, most recently
from
ef3a0a1
to
339a6f4
Compare
| @@ -13,7 +13,11 @@ export const getUser = state => { | |||
| return state.user | |||
| } | |||
|
|
|||
| export const getUserName = state => { | |||
There was a problem hiding this comment.
this line has to be moved to 339a6f4
idali0226
force-pushed
the
ida/implement-start-page
branch
from
339a6f4
to
994ebae
Compare
No description provided.