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

Card, Index and app.js [Any logic code affected] update #22

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

patodm
Copy link

@patodm patodm commented Oct 3, 2021

Hi! @vinitshahdeo this is my update proposal:

  • Desktop Mockup 1
    Desktop Mockup 1

  • Desktop Mockup Showing hover effect
    Desktop Mockup Showing hover effect

  • Mobile Mockup

Mobile Mockup

  • Mobile Mockup Showing Sticky Selector

Mobile Mockup Showing Sticky Selector

And here my changes:

  • I added a div element with the class "news-selector-wrapper" as wrapper for the select element.
<div class="news-selector-wrapper">
    <select id="news-selector"></select>
</div>
  • I added 3 new divs element as wrapper in createArticle function of app.js [Important: It was neccesary to add these classes for add new hover effects for the card, and I did not touch any part of logic in that function].
    Div elements created:
  1. div with "header-wrapper" class: As wrapper for header section that includes image and title of card.
  2. div with "headline" class: The headline class initially was in p tag, but to have more freedom over the element and over the global style I changed it to the new div.
  3. div with "body-wrapper" class: As wrapper for body section that includes author and description of card.
function createArticle(article) {
  return `
    <a class="story" href="${article.url}">
      <div class="header-wrapper">
        <img class="story-image" src="${article.urlToImage}" alt="${article.title}">
        <div class="headline">
          <p>${article.title}</p>
        </div>
      </div>
      <div class="body-wrapper">
        <p class="author">${article.author ? article.author : ""}</p>
        <p class="description">${article.description}</p>
      </div>
    </a>
  `;
}
  • Finally I updated the style.css:
  1. Added root for global styles, so the code will look more clean and without repeated elements.
  2. Update original classes to adapt to the new ones.
  3. Added responsivness thanks to add display: grid and related styles to story-section, which is the wrapper of all the cards on screen.
  4. I added sticky style for the select tag with class "news-selector", of that way always will be at top of the website working as a good UX practice.
  5. Changed main color.
  6. Extra: Added a Table of contents to have every section organized.

@welcome
Copy link

welcome bot commented Oct 3, 2021

Thanks for opening this pull request! 🤗
Wishing you a great Hacktoberfest 2020 🙌 🎉 ⚡️
Are you looking for more beginner-friendly issues? Check out this repo.

Hacktoberfest2020

📢 Spread the word about @vinitshahdeo/Hacktoberfest2020 repo across your social media channels to help get others involved!
Give a shoutout to DEV article. Retweet this.

Check out few other repos below 🚀

PortScanner
SimpleBio
HBD
Map Of India

Show some ❤️

  • Consider leaving a ⭐ here.
  • Check out more beginner-friendly repos here.
  • Follow @vinitshahdeo for more updates.
  • Read my open-source journey here.

Say Hi on Twitter! 👋

Twitter Follow

PS: Please add @vinitshahdeo as a reviewer if you haven't added.

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.

1 participant