Skip to content

S1 F1 - #5

Open
RachmannJoubert wants to merge 39 commits into
story1from
S1F1
Open

S1 F1#5
RachmannJoubert wants to merge 39 commits into
story1from
S1F1

Conversation

@RachmannJoubert

Copy link
Copy Markdown
Owner

.Create and display table

Comment thread functions.php Outdated
Comment thread index.php Outdated
Comment thread index.php Outdated
Comment thread functions.php Outdated
Comment thread index.css
Comment thread index.css
Comment thread index.css Outdated
Comment thread functions.php Outdated
Comment thread index.css
Comment thread index.css Outdated
Comment thread styles/index.css
Comment thread styles/index.css Outdated
Comment thread styles/index.css Outdated
Comment thread styles/index.css
Comment thread test/functionsTest.php Outdated
@@ -0,0 +1,11 @@
<?php

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Functions not currently tested

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

I've completed success test, will begin working on failure and malform test.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

2 more to go

Comment thread styles/index.css Outdated
Comment thread styles/index.css Outdated
Comment thread test/functionsTest.php Outdated
@@ -0,0 +1,11 @@
<?php

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

2 more to go

Comment thread test/functionsTest.php Outdated
Comment thread test/functionsTest.php Outdated
Comment thread test/functionsTest.php Outdated
@mallowmew

Copy link
Copy Markdown

I don't think your picture numbers match up with the IDs shown in your table? This isn't a chinese chestnut.
Screenshot 2019-08-08 at 16 37 53

Comment thread functions.php

function processData(array $plants): string
{

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

extra newline

Comment thread functions.php
if (array_keys($images) !== range(0, count($images) - 1))
return false;


Copy link
Copy Markdown

Choose a reason for hiding this comment

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

extra newline

Comment thread functions.php
function displayImage(array $images): string {

if (array_keys($images) !== range(0, count($images) - 1))
return false;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Indent this.
Also, you don't need the curlyboys for a one-liner if statement, but they do help readability.

Comment thread index.php
<link rel="stylesheet" type="text/css" href="index.css">
<link rel="stylesheet" type="text/css" href="normalize.css">
<link rel="stylesheet" type="text/css" href="styles/index.css">
<link rel="stylesheet" type="text/css" href="styles/normalize.css">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Normalize should come before your own CSS, because the browser will overwrite rules in downwards-order. You want your own rules to override normalize, not the other way around.

Comment thread index.php
?>

<!DOCTYPE html>
<html lang="eng">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Correct html lang attribute is en

Comment thread index.php Outdated
<li>Life Span</li>
</ul>
</div>
<?php echo processData($plants) ?>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Charlie advised that the only functionality you should have in php tags within the html body is echoing string variables you already made in the php block. This method call should go up in the php block and load its message into a string, which you then echo here. This keeps your 'doing stuff' code all together.

Comment thread styles/index.css
}

.row {
width: 100%;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Doesn't seem to do anything.

Comment thread styles/index.css
ul {
padding-top: 3%;
border-top: solid black;
width: 100%;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Doesn't seem to do anything.

@FelixLetheren FelixLetheren left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

bad

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.

6 participants