Skip to content
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

added popular poetry section #21

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

Conversation

KMcWeeney22
Copy link
Collaborator

The work is this branch is related to Issue #8. One of the main sections were changed to feature links to 3 popular poems with poetry art related to that poem. Right now its just a link with a picture but should there be any other information to go along with each featured poem?

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Maybe we can list other poetry by those poets?

@hlkessner
Copy link
Collaborator

I think this branch looks good to merge with the master. There are a few little tweaks we can implement later in development, but the code is sound and won't conflict with anything in our master code.

Copy link
Collaborator

@lariosw lariosw left a comment

Choose a reason for hiding this comment

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

I thought you did a really good job with this section and how you laid it out. I think that if this section had the ability to rotate popular poets in a randomized way that would make this feature be great in a final stage.

<ul class="poetry-list" style="list-style: none; padding-left: 0px;">
<li style="padding-bottom: 20px;">
<a href="https://www.poetryfoundation.org/poems-and-poets/poems/detail/45521">I Wandered Lonely as a Cloud</a>
<p><img class ="img-responsive" src="http://t0.gstatic.com/images?q=tbn:ANd9GcRa6CA5UmvvNVVWN6YSCa_TuO2I66V-HWjGhBEwzRyA3sRbD-41" alt="I wandered lonely as a cloud cover photo" style="height: 100px;"></p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Photo layout looks good. Only suggestion would be that class="img-responsive" might be a too generic of name for these photos and it might be better to name it something like poet-image with an id of walt whitman for example. I was thinking this might be a good change since there will be many photos we will use that will be responsive for the profile pictures and possibly other photos within the site.

<ul class="poetry-list" style="list-style: none; padding-left: 0px;">
<li style="padding-bottom: 20px;">
<a href="https://www.poetryfoundation.org/poems-and-poets/poems/detail/45521">I Wandered Lonely as a Cloud</a>
<p><img class ="img-responsive" src="http://t0.gstatic.com/images?q=tbn:ANd9GcRa6CA5UmvvNVVWN6YSCa_TuO2I66V-HWjGhBEwzRyA3sRbD-41" alt="I wandered lonely as a cloud cover photo" style="height: 100px;"></p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

<p><img class ="poet-images" id="walt-whitman" src="http://t0.gstatic.com/images?q=tbn:ANd9GcRa6CA5UmvvNVVWN6YSCa_TuO2I66V-HWjGhBEwzRyA3sRbD-41" alt="I wandered lonely as a cloud cover photo" style="height: 100px;"></p>

</div>
<!-- end popular poetry list -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice commenting out the section. Helps keep it clear where these changes end.

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.

None yet

3 participants