Skip to content

Comments

S1 F1#5

Open
RachmannJoubert wants to merge 39 commits intostory1from
S1F1
Open

S1 F1#5
RachmannJoubert wants to merge 39 commits intostory1from
S1F1

Conversation

@RachmannJoubert
Copy link
Owner

.Create and display table

@@ -0,0 +1,11 @@
<?php

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
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.

Choose a reason for hiding this comment

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

2 more to go

@@ -0,0 +1,11 @@
<?php

Choose a reason for hiding this comment

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

2 more to go

@mallowmew
Copy link

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


function processData(array $plants): string
{

Choose a reason for hiding this comment

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

extra newline

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


Choose a reason for hiding this comment

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

extra newline

function displayImage(array $images): string {

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

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.

<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">

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.

?>

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

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

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

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.

}

.row {
width: 100%;

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.

ul {
padding-top: 3%;
border-top: solid black;
width: 100%;

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.

Copy link

@FelixLetheren FelixLetheren left a comment

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