-
Notifications
You must be signed in to change notification settings - Fork 1
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
base: master
Are you sure you want to change the base?
Conversation
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.
Maybe we can list other poetry by those poets?
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. |
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 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> |
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.
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> |
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.
<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 --> |
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.
Nice commenting out the section. Helps keep it clear where these changes end.
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?