Skip to content
This repository has been archived by the owner on Jan 30, 2018. It is now read-only.

Update Parents_content.php #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ZodiacChaser
Copy link

Want to add section with students sheets they need to join the team. (This may be utter crap I've never really used HTML before)

Want to add section with students sheets they need to join the team. (This may be utter crap I've never really used HTML before)
@murraybd
Copy link
Contributor

murraybd commented Sep 1, 2015

The change itself to the Parents_content.php page is mostly fine, however there is one large problem in that nothing links to or displays that page any more. If we want to display that page again it would need to be added to the "$menu" array in index.php. Although it doesn't fit well in any of the existing categories.

I'll make another comment about the file links in-line.

@murraybd
Copy link
Contributor

murraybd commented Sep 1, 2015

Thanks for working on this!

<p class="byline"><small>Posted on August 24th, 2015</small></p>
<div class="entry">
<p>
Please fill out the below papers and bring them with you to the next open shop night: </br>

Choose a reason for hiding this comment

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

it is a 👃 to use <br> also the the slash is not needed for html5 (and it is on the wrong side anyway)
it would be better to wrap the text node in the <p> block, and just throw your <ul> block below it.
You NEVER should leave text nodes as peers to html elements. The render cycle has a super bad time when flowing the document.

I would suggest a structure like:

<div class="entry">
  <p>
    Please fill out the below papers and bring them with you to the next open shop night:
  </p>
  <ul class="papers">
    <li class="papers__item"><a target="_blank" href="some-url">Some Text</a></li>
    <li class="papers__item"><a target="_blank" href="some-url">Some Text</a></li>
    <li class="papers__item"><a target="_blank" href="some-url">Some Text</a></li>
  </ul>
</div>

Structuring your html this way is good for the soul AND the flow cycle.

Choose a reason for hiding this comment

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

And this is an old PR...man oh man...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants