-
Notifications
You must be signed in to change notification settings - Fork 285
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
Webpage in master 1 #78
Webpage in master 1 #78
Conversation
5b98b70
to
618b5bf
Compare
618b5bf
to
388c9db
Compare
This is good to merge. What is inside:
TODO: remove webpage branch after merge |
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.
Thank you very much, @stereobooster.
It looks great overall, I just left a couple of questions/comments.
webpage/scripts/generate_readme.js
Outdated
@@ -0,0 +1,51 @@ | |||
#!/usr/bin/env node |
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.
Should we remove this file from the current 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.
I will remove last commit from this PR
webpage/src/components/Table.js
Outdated
|
||
const CellTd = options => { | ||
const { header, value } = options; | ||
if (header === "Package") { |
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 recurring pattern (if Package else if Version) doesn't seem right.
Do you mind implementing another solution?
Like using a map, or different components.
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.
const fields = {
"Package": value => { ... return join(links, " + "); },
"Version": x => x,
"default": symbol
}
const field = fields[header] || fields["default"]
return <td>{ field(value) }</td>
webpage/src/components/Table.js
Outdated
}, []); | ||
|
||
const symbol = value => { | ||
if (value === true) { |
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.
What are the possible values of value
here?
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.
true
, false
, undefined
f385e7d
to
72414b0
Compare
@MicheleBertoli done |
LGTM, thank you very much @stereobooster. |
Yay 0e680fe |
Probably missed it when was moving a website from one branch to another. |
No description provided.